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

Track contexts in hooks as state #787

Merged
merged 13 commits into from Jul 14, 2022
Merged

Track contexts in hooks as state #787

merged 13 commits into from Jul 14, 2022

Conversation

rmorshea
Copy link
Collaborator

@rmorshea rmorshea commented Jul 11, 2022

Description

closes: #789

components which are newly added to the layout after the initial render do not have access to contexts. we solve this by tracking life cycle hooks in stack and copying the context providers from parent to child hooks.

Checklist:

Please update this checklist as you complete each item:

  • Tests have been included for all bug fixes or added functionality.
  • The changelog.rst has been updated with any significant changes.
  • GitHub Issues which may be closed by this Pull Request have been linked.

@rmorshea rmorshea changed the title fix use_context Track contexts in hooks as state Jul 13, 2022
components which are newly added to the layout after the initial
render do not have access to contexts. we solve this by tracking
life cycle hooks in stack and copying the context providers from
parent to child hooks.

we also change how contexts are implemented - instead of needing
a create_context function which returns a new Context class, we
just return a Context object that constructs a ContextProvider
component. the earlier implementation was too clever and did not
add anything for it.
@rmorshea
Copy link
Collaborator Author

@Archmonger, thoughts on removing create_context in favor of a simple Context object constructor?

@Archmonger
Copy link
Contributor

I am completely indifferent to that change. At the moment, you and I are the only ones using it. And I don't expect many people to create custom context hooks for IDOM in the future.

@rmorshea
Copy link
Collaborator Author

I decided not to do that. Came up with a nice solution using functions. I'm going to remove the name parameter of create_context though. React doesn't have it, and I'm not sure it provides that much value.

@rmorshea rmorshea force-pushed the fix-use-context-issue branch 4 times, most recently from 61b4dd6 to 5174ec4 Compare July 14, 2022 05:22
@rmorshea rmorshea merged commit 7bebc4d into main Jul 14, 2022
@rmorshea rmorshea deleted the fix-use-context-issue branch July 14, 2022 23:46
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.

Conditionally Rendered Components Cannot Have Context
2 participants