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

observedBits 32bit limitation #1

Closed
dai-shi opened this issue Nov 13, 2018 · 7 comments
Closed

observedBits 32bit limitation #1

dai-shi opened this issue Nov 13, 2018 · 7 comments

Comments

@dai-shi
Copy link
Owner

dai-shi commented Nov 13, 2018

Discussion from https://link.medium.com/gXvqudhDNR

@dai-shi
Copy link
Owner Author

dai-shi commented Nov 13, 2018

Option 1: we rotate the index. % 31
Option 2: we use multiple contexts for each 31 items.
Option 3: multiple contexts for each items. Do not rely on observedBits

@cyphercider
Copy link

Assuming you abstract the implementation away from the user, I guess the decision criteria are (?)

  1. Potential high overhead or general “misuse” of the react context feature if potentially hundreds of contexts are created as with option API design question #3
  2. Rendering efficiency of the app to the extent that with observedBits 32bit limitation  #1, some components will undesirably receive a state change when an overlapping index has changed

I don’t know enough about the implementation of Context in the react codebase to know if #1 is a concern.

It’s still not clear to me how we may subscribe to deep state or if that is even desirable considering we can receive a state change for the high level state but avoid re renders using things like memo() for functional components and useMemo for computed values.

@dai-shi
Copy link
Owner Author

dai-shi commented Nov 14, 2018

Thanks. I'm not sure either how much overhead creating hundreds of contexts leads.
Another concern is whether observedBits will ever be official...

BTW, https://github.com/dai-shi/react-context-global-state is already taking the Option 3 approach.

@dai-shi
Copy link
Owner Author

dai-shi commented Nov 14, 2018

but avoid re renders using things like memo()

something like this?

11_deep / Person.tsx

const PersonFirstName = () => {
  const [value] = useGlobalState('person');
  const { firstName } = value;
  return useMemo(
    () => (
      <div>
        First Name:
        <TextBox text={firstName} />
        <input
          value={firstName}
          onChange={setFirstName}
        />
      </div>
    ),
    [firstName]);
};

@cyphercider
Copy link

Yeah, that's exactly what I had in mind for useMemo.

@dai-shi
Copy link
Owner Author

dai-shi commented Nov 15, 2018

Option 4: we pass non-updating "store" in the context, and let hooks subscribe for key-based changes. This is mostly like the previous implementation which requires useState.

@dai-shi
Copy link
Owner Author

dai-shi commented Nov 17, 2018

Turns out that the current implementation already takes the option 1 approach.
I keep it as is, otherwise the library implementation gets much bigger.
This limitation should be clearly noted in README.md.

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

No branches or pull requests

2 participants