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

Context.write #89

Closed
wants to merge 2 commits into from
Closed

Context.write #89

wants to merge 2 commits into from

Conversation

acdlite
Copy link
Member

@acdlite acdlite commented Nov 20, 2018

This is a very early stage proposal. I'm opening this RFC as a way to explain the problem space. We're still exploring alternative solutions. If this were an ECMAScript proposal it'd be Stage 0.

Summary

Proposes an extension to the context API for updating the default context value. This would allow for React-managed state that lives outside the UI tree and is shared across roots.

Basic example

const Context = React.createContext(initialValue, contextDidUpdate);
// Update the global context value across all roots. Any context consumer that
// is not wrapped in a Provider will re-render with this value.
Context.write(newValue);
// Functional updates also supported for access to the previous value.
Context.write(prevValue => newValue);

function contextDidUpdate(newContext) {
  // Optional callback that fires whenever context changes
}

Rendered text

@theKashey
Copy link

For each context, React would have to maintain a separate version per root, as well as a separate queue of updates.

I was just thinking about it - per-root changeable defaultValue would allow "automatic Provider injection" effect, making some stuff(like React-Helmet, side-effect, etc) easier.

@gaearon
Copy link
Member

gaearon commented Nov 20, 2018

We could also use this to store latest versions of component implementations in hot reloading, and instead of forceUpdate we'd update the context.


## How to handle multiple roots

The trickiest question how to deal with multiple roots. React does not guarantee consistency across roots; each root has its own commit phase, and suspending inside one root has no effect on the others. This isn't observable in synchronous mode, since React will block the main thread (including paint) until every root's commit phase has finished. In concurrent mode, however, React may yield in between each commit. With Suspense, the time between each commit phase could vary by many seconds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The trickiest question is how"

@acdlite acdlite force-pushed the context-write branch 2 times, most recently from 7d96361 to d3dd7be Compare November 20, 2018 22:29
@acdlite acdlite changed the title [wip] Context.write proposal Context.write Nov 20, 2018

Still, although caching in component state isn't ideal, it's arguably "good enough" for many use cases today.

Not so with Suspense. The Suspense model is that the first time React renders an IO-bound component, an exception is thrown when attempting to read the data. While React is waiting for the data to load, it continues rendering the rest of the tree, but it doesn't commit the result; the partially completed tree is _discarded_. Once the data has resolved, React attempts to render the entire tree over again from scratch. It may, as an optimiztion, reuse parts of the previous attempt. But it's not a guarantee. That means using local component state to cache data won't work, because that state will most likely not be persisted. The underlying priciple here is that you can't cache the intermediate results of a computation (server data) on the output of the computation itself (the React tree).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The underlying principle"

@tf
Copy link

tf commented Nov 21, 2018

In particular, consumer components could still be used with a provided context to replace the global default context with a fixture in tests, right?

@jfo84
Copy link

jfo84 commented Nov 21, 2018

Great job @acdlite :)


An alternative model is to treat all the roots as siblings and commit them at the same time. This could be viable for single page React apps (where there aren't that many roots, anyway). It doesn't work so well in cases where React is embedded inside another framework (e.g. progressive enhancement of a server rendered app), where temporary inconsistencies may be desirable. For example, the opt-in API for concurrent mode relies on roots committing separately so that you can upgrade some roots to concurrent mode without upgrading the entire app. In an app with mixed synchronous and concurrent roots, a unified commit would mean that every call to `Context.write` has to be synchronous, which probably makes this option a non-starter.

In either of these models, React would need to track a global list of all roots in order to schedule updates on them. This isn't something we do currently, and it means roots would no longer be automatically garbage collected; discarded roots would need to be explicitly unmounted to remove them from the global list.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If by "discarded roots" you mean those which are just removed from DOM without calling unmountComponentAtNode(), why not use MutationObserver?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would that work? AFAIK MutationObserver only works for immediate children. It won't tell you if a tree is disconnected by an ancestor node.


### Immutable store (like React Redux)

Implementing immutable stores is straightfoward. Actions are applied in the render phase using the functional form of `Context.write`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a point I'm misunderstanding. What does it mean concretely that

Actions are applied in the render phase using the functional form of Context.write?

Copy link
Member Author

@acdlite acdlite Nov 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanation of render phase versus commit phase: https://twitter.com/acdlite/status/977291318324948992


This doesn't address the multiple root problem, nor does it work with React's server renderer, which does not have updates.

## Use Hooks

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for more symmetry between component state and context. Ideally, lifting component state & logic up to into a shared context wouldn't require rewriting the whole thing.

Copy link

@diegohaz diegohaz Nov 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I've been doing stuff on this direction on https://github.com/diegohaz/constate

Ideally, we should be writing local state until we really need to lift it up. Then, refactoring it into global/shared/contextual state should be something really simple.

@rybon
Copy link

rybon commented Dec 24, 2018

@acdlite fantastic proposal!

What's the plan? Where do we go from here? Is there a concrete path forward like there is for the Hooks proposal?

@zombieJ
Copy link

zombieJ commented Apr 9, 2019

Is that possible to add freeze like:

const Context = React.createContext(initialValue, contextDidUpdate);
Context.freeze();

Context.write(newValue); // Error. This context has been freeze and can't change defaultValue.

@dancerphil
Copy link

dancerphil commented Jun 24, 2019

implement in https://codesandbox.io/s/react-context-next-xk1rk using redux

@jamesplease jamesplease mentioned this pull request Nov 9, 2019
@mucsi96
Copy link

mucsi96 commented Nov 9, 2019

I have submitted an RFC with alternative solution. I believe more inline with hooks direction.
#130
Would like to ask for feedback from all of you :)

@gaearon
Copy link
Member

gaearon commented Feb 10, 2020

Looks like this either needs more work and/or won't work at all.

@gaearon gaearon closed this Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.