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

[RFC] Idea: forceDeepUpdate() and forceDeepUpdateWithScope(scope) #7759

Closed
sebmarkbage opened this issue Sep 16, 2016 · 10 comments
Closed

[RFC] Idea: forceDeepUpdate() and forceDeepUpdateWithScope(scope) #7759

sebmarkbage opened this issue Sep 16, 2016 · 10 comments

Comments

@sebmarkbage
Copy link
Collaborator

Just going to put it out there for feedback...

Motivation

Subscription management comes with a cost and that eats into the wins of async rendering since it needs to be managed synchronously. Not just managing the direct subscriptions themselves but managing the dynamic dependency graph so that it can be invalidated.

Meanwhile, most of what subscriptions are used for is data that will never update. At least in our apps. It is a pure loss.

The use case is when you're connecting to third party systems that aren't as easily connected to the top level data tree.

Proposal

this.forceDeepUpdate();

Same use case as forceUpdate, if you are reading from global mutable state for some reason, you can use this to by-pass shouldComponentUpdate in an entire subtree. Basically rerender everything. When combined with Fiber this can be a low-priority update so it's not so bad for things that change a lot of things.

A good example would be changing the locale. Regardless if you read a global mutable locale (like AirBnB does) or a context locale (like Yahoo) does, this lets you change it when you need to. Without needing to manage subscriptions for all those cases when you don't need it.

this.forceDeepUpdateWithScope(scope);
class Foo extends React.Component {
  shouldComponentUpdateForScope(scope) {
    return scope.store === UserStore && scope.id === this.props.userID;
  }
  render() {
    ...
  }
}

forceDeepUpdateWithScope would traverse the subtree and only start rendering if shouldComponentUpdateForScope returns true for the arbitrary scope argument. This allows for a bit more of a targeted update with some convenience overhead.

Additionally, React would cache the pair of scope and components that responded. For some number of scopes back. If a new component gets mounted with a shouldComponentUpdateForScope we might check it against the cache to see if we need to add it to the cache.

Effectively this creates lazy subscriptions.

The use case is something like typing into an input field that then updates some global store which immediately displays in a completely different place on the page. The first character might be a bit slower but still with responsive levels and the subsequent characters are fast to update.

Caveat

The major downside of this proposal is that it relies on mutation. As we know, React doesn't really like mutation for many more reasons than just shouldComponentUpdate.

The effect in Fiber for example, is that any component that gets a higher priority update will start using the new value. Components that rely on mutable state effectively become up-prioritized which is not good. 1) It can temporarily show inconsistent data. 2) The point of making this kind of update lower priority is because it is likely to be large. Larger updates will stall the page if they take the same priority as higher priority updates. Thereby defeating the benefits of Fiber anyway.

I'd like to try to come up with a variant of this API that doesn't rely on mutation.

@developit
Copy link
Contributor

@sebmarkbage Sorry if this is naive but what is scope referring to here?

@sebmarkbage
Copy link
Collaborator Author

@developit It is some kind of user provided tuple. It can be anything that we can implement an equality check on I guess. I'd expect it to be something like const locale = Symbol(); to indicate that the scope is a "locale" change or a tuple of "store" and "id" for more classic Flux patterns.

@gaearon
Copy link
Collaborator

gaearon commented Sep 17, 2016

I need this for hot reloading too.

@karczk
Copy link
Contributor

karczk commented Sep 18, 2016

I cannot agree with this concept in general, but localization is a good example. Scoping for updates can be done with immutable state (new reference on proper level). Everything should by pass by props, but I don't want to add locale to all interfaces and repeat assignment. For localization, I use global field (window._currentCulture) and when language is changed I call _.deepClone() (from lodash lib) on main state (in store), forcing all components to refresh. I'm not proud of it, but this is my workaround (only for this situation). I'm curious how you deal with it?

@syranide
Copy link
Contributor

syranide commented Sep 18, 2016

I'd like to try to come up with a variant of this API that doesn't rely on mutation.

Intuitively wouldn't that be to introduce a new life-cycle method that is called before next render after forceDeepUpdate() is called (it should be renamed then)? So that the component can read and store the relevant global values in state on construction, making rendering consistent, and then you have the new life-cycle method which allows you to refresh state with the new data from the global variables. I.e. componentWillUpdate but only after forceDeepUpdate (again, rename). This would allow you to keep the normal behavior of rendering and shouldComponentUpdate will still work, keeping it performant.

So in the end there would be 3 ways of passing data, which would basically exists on an axis;

  1. Explicitly passing props to a child (props).
  2. Tunneling data from a parent to children in a sub-tree (context:ish).
  3. Refreshing global data for all instances in a tree (this).

All three have their strength and weaknesses, but each with use-cases where they are the best solution to the problem.

EDIT: I would go so far as to say that explicitly passing e.g. locale through the hierarchy as a prop is not just counter-productive, it's the wrong solution. It's technically wasteful and practically cumbersome, refactoring becomes a nightmare and you end up having to pass it to all components regardless of whether or not they or their descendants actually need it (because you never know if they want or will want it).

@sebmarkbage
Copy link
Collaborator Author

@syranide That's an interesting idea to put this in state. I was thinking that we could also have a different API like getData() which updates this.data (kind of like the previous observe idea). So that this third thing gets its own field which can be restored on demand. This derived value would be immutable.

@syranide
Copy link
Contributor

syranide commented Sep 19, 2016

That's an interesting idea to put this in state. I was thinking that we could also have a different API like getData() which updates this.data (kind of like the previous observe idea).

@sebmarkbage I didn't really think too deeply about state vs data, was mainly trying to keep it simple for the discussion. But yeah, I mean it is fundamentally props, just a different way of getting them, so if you also have a life-cycle method like componentWillReceiveProps but for data then it allows you to precompute expensive stuff you ever feel it necessary (like we allow for props), that's probably the best of both worlds. So yeah, intuitively getData() seems like a better decision.

As a thought experiment; what if data AND contexts were all merged into props. So you would have a single interface to get your props, regardless of source, you could then override globals with contexts, and contexts with props. Practically you would need to specify these "bridges" explicitly, i.e. "expose locale to components of type X, Y and Z." (with some way tagging components it would be easy to target groups of components which expect the same data). However, this means that it is totally/virtually transparent to descendants and props-vs-subtrees-vs-globals becomes a local decision rather than an explicit part of a components' interface.

@vitalets
Copy link
Contributor

vitalets commented Oct 3, 2017

Hi, any update on this?
It also would be helpful for changing global theme of app.

@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

If there are updates they will be on this issue 😉
We might look at it again in a few months.

@gaearon
Copy link
Collaborator

gaearon commented Jan 2, 2018

Seems like we went with this as an alternative. reactjs/rfcs#2

@gaearon gaearon closed this as completed Jan 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants