Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hooks] Allow changing cursor path dynamically #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rasendubi
Copy link

With the current implementation, when cursors changes, a new watcher
is created and we subscribe to "update" event. The issue is, if the
value under new path never changes, useBranch will always return the
value from the previous path.

In example below (from documentation), changing alternative prop
will not change colors after first render.

import React, {Component} from 'react';
import {useBranch} from 'baobab-react/hooks';

const List = function(props) {
  // Using a function so that your cursors' path can use the component's props etc.
  const {colors} = useBranch({
    colors: [props.alternative ? 'alternativeColors' : 'colors']
  });

  function renderItem(color) {
    return <li key={color}>{color}</li>;
  }

  return <ul>{colors.map(renderItem)}</ul>;
}

export default List;

Fix this by re-fetching the current value in the useEffect.

Another issue is that with the current examples, selectors changes
on every render. Passing in an object or function inline will create a
new instance of object on every render, triggering watcher
re-creation.

With the fix above, that leads to an infinite loop:

  • useEffect sets new value for state
  • updating state triggers a render
  • render creates a new selectors triggering useEffect

Fix this by adding a deps argument that should hold an array of
dependencies to trigger re-subscription. The default value is now an
empty array to break the infinite loop and be more
backward-compatible.

Another alternative is to use useMemo to memoize selectors in
examples, but that is overly-verbose and would require a lot of
migration effort for users.

With the current implementation, when `cursors` changes, a new watcher
is created and we subscribe to "update" event. The issue is, if the
value under new path never changes, `useBranch` will always return the
value from the previous path.

In example below (from documentation), changing `alternative` prop
will not change `colors` after first render.

```js
import React, {Component} from 'react';
import {useBranch} from 'baobab-react/hooks';

const List = function(props) {
  // Using a function so that your cursors' path can use the component's props etc.
  const {colors} = useBranch({
    colors: [props.alternative ? 'alternativeColors' : 'colors']
  });

  function renderItem(color) {
    return <li key={color}>{color}</li>;
  }

  return <ul>{colors.map(renderItem)}</ul>;
}

export default List;
```

Fix this by re-fetching the current value in the `useEffect`.

Another issue is that with the current examples, `selectors` changes
on every render. Passing in an object or function inline will create a
new instance of object on every render, triggering watcher
re-creation.

With the fix above, that leads to an infinite loop:

- `useEffect` sets new value for `state`
- updating `state` triggers a render
- render creates a new `selectors` triggering `useEffect`

Fix this by adding a `deps` argument that should hold an array of
dependencies to trigger re-subscription. The default value is now an
empty array to break the infinite loop and be more
backward-compatible.

Another alternative is to use `useMemo` to memoize selectors in
examples, but that is overly-verbose and would require a lot of
migration effort for users.
@Yomguithereal
Copy link
Owner

@arnaudmolo any insight? I am a bit out of the water with hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants