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

Provide more ways to bail out inside Hooks #14110

Open
gaearon opened this Issue Nov 5, 2018 · 110 comments

Comments

Projects
None yet
@gaearon
Copy link
Member

gaearon commented Nov 5, 2018

There's a few separate issues but I wanted to file an issue to track them in general:

  • useState doesn't offer a way to bail out of rendering once an update is being processed. This gets a bit weird because we actually process updates during the rendering phase. So we're already rendering. But we could offer a way to bail on children.
  • useContext doesn't let you subscribe to a part of the context value (or some memoized selector) without fully re-rendering.
@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Nov 5, 2018

cc @markerikson you probably want to subscribe to this one

@markerikson

This comment has been minimized.

Copy link

markerikson commented Nov 5, 2018

Yay! Thanks :)

@alexeyraspopov

This comment has been minimized.

Copy link
Contributor

alexeyraspopov commented Nov 5, 2018

useContext doesn't let you subscribe to a part of the context value (or some memoized selector) without fully re-rendering.

useContext receives observedBits as a second param. Isn't it the same?

@gaearon

This comment has been minimized.

Copy link
Member

gaearon commented Nov 5, 2018

I guess you're right the context one is an existing limitation (ignoring the unstable part).

@markerikson

This comment has been minimized.

Copy link

markerikson commented Nov 5, 2018

@alexeyraspopov : nope! Here's an example:

function ContextUsingComponent() {
    // Subscribes to _any_ update of the context value object
    const {largeData} = useContext(MyContext);
    
    // This value may or may not have actually changed
    const derivedData = deriveSomeData(largeData);
    
    // If it _didn't_ change, we'd like to bail out, but too late - we're rendering anyway!
}

observedBits is for doing an early bailout without actually re-rendering, which means you can't locally do the derivation to see if it changed.

As an example, assuming we had some magic usage of observedBits in React-Redux:

Imagine our Redux state tree looks like {a, b, c, d}. At the top, we calculate bits based on the key names - maybe any change to state.b results in bit 17 being turned on. In some connected component, we are interested in any changes to state.b, so we pass in a bitmask with bit 17 turned on. If there's only a change to state.a, which sets some other bit, React will not kick off a re-render for this component, because the bitmasks don't overlap.

However, while the component is interested in changes to bit 17, it still may not want to re-render - it all depends on whether the derivation has changed.

More realistic example: a user list item is interested in changes to state.users, but only wants to re-render if state.users[23] has changed.

@sam-rad

This comment has been minimized.

Copy link

sam-rad commented Nov 5, 2018

Perhaps a possible api would be:

function Blah() {
  // useContext(context, selectorFn);
  const val = useContext(Con, c => c.a.nested.value.down.deep);
}

And shallowEqual under the hood.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Nov 5, 2018

@snikobonyadrad: won't work - the second argument is already the observedBits value:

export function useContext<T>(
  Context: ReactContext<T>,
  observedBits: number | boolean | void,
)

@gaearon : semi-stupid question. Given that returning the exact same elements by reference already skips re-rendering children, would useMemo() kinda already solve this?

function ContextUsingComponent() {
    const {largeData} = useContext(MyContext);
    const derivedData = deriveSomeData(largeData);
    
    const children = useMemo(() => {
        return <div>{derivedData.someText}</div>
    }, [derivedData]);
}
@sophiebits

This comment has been minimized.

Copy link
Contributor

sophiebits commented Nov 6, 2018

@markerikson Yes, but that means that ContextUsingComponent needs to know about this, even if you might otherwise want to put the two useContext+derive calls into a custom Hook.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Nov 6, 2018

Yeah, I know, just tossing it out there as a sort of semi-stopgap idea.

Any initial thoughts to what a real API like this might look like?

@Kovensky

This comment has been minimized.

Copy link

Kovensky commented Nov 8, 2018

Crazy idea: add React.noop as a reconciler-known symbol, throw React.noop;

Not sure how that would mesh with this interrupting further hooks from running, and there is already a problem with the reconciler throwing out hooks that did already run before a component suspends.

@ioss

This comment has been minimized.

Copy link
Contributor

ioss commented Nov 8, 2018

I personally don't like noop, as I would expect it to do nothing. :)
How about React.skipRerender or React.shouldComponentUpdate(() => boolean | boolean) or similar?

Also, it should be a call: React.whateverName(), which could then do whatever is needed (probably throw a marker, as you suggested) and especially ignore the call on the first render, which should probably not be skipped.

I also thought about the possibility to return early in the render method with a marker (React.SKIP_UPDATE), but that wouldn't work in custom hooks. On the other hand, skipping rerendering in custom hooks might be strange? What do you think?

@dai-shi

This comment has been minimized.

Copy link

dai-shi commented Nov 15, 2018

Hi,
I'm experimenting a new binding of Redux for React.
For now, I use a workaround for bailing out.
Does the scope of this issue cover this use case?
https://github.com/dai-shi/react-hooks-easy-redux

@brunolemos

This comment has been minimized.

Copy link

brunolemos commented Nov 19, 2018

I would enjoy something like this to avoid unnecessary re-renders:

const { data } = useContext(MyContext, result => [result.data])

where the second parameter would work like the second parameter of useEffect, except it's () => [] instead of [], with result being the response of useContext(MyContext).

Note: This is supposing the existing second param could change or react internally could check the typeof to see if it's a function or the observedBits

@slorber

This comment has been minimized.

Copy link
Contributor

slorber commented Nov 19, 2018

Hi,

I would also like the same api as @brunolemos describes, for using it with tools like Unstated which I use as a replacement for Redux store with a similar connect() hoc currently.

But I think there is ambiguity in your API proposal @brunolemos not sure exactly what happens if you return [result.data, result.data2] for example. If you return an array you should probably assign an array to the useContext result too?

Not sure exactly how observedBits works and could be helpful here, anyone can explain? If we have to create an observedBits it should probably be derived from the context value data slice we want to read no? so current api does not seen to do the job for this usecase.

What if we could do const contextValue = useContext(MyContext, generateObservedBitsFromValue)?

@gnapse

This comment has been minimized.

Copy link

gnapse commented Nov 19, 2018

won't work - the second argument is already the observedBits value:

@markerikson how official is this second argument? I see that it is not documented publicly yet.

I mention this because the api proposal mentioned by @sam-rad in this comment is what I was expecting it to be eventually, to solve the partial subscribing to context.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Nov 19, 2018

@slorber : See these links for more info on observedBits:

@gnapse : The React team has said a couple times that they're not sure they want to keep around the observedBits aspect, which is why the Context.Consumer's prop is still called unstable_observedBits.

@TotooriaHyperion

This comment has been minimized.

Copy link

TotooriaHyperion commented Nov 20, 2018

@snikobonyadrad: won't work - the second argument is already the observedBits value:

export function useContext<T>(
  Context: ReactContext<T>,
  observedBits: number | boolean | void,
)

@gaearon : semi-stupid question. Given that returning the exact same elements by reference already skips re-rendering children, would useMemo() kinda already solve this?

function ContextUsingComponent() {
    const {largeData} = useContext(MyContext);
    const derivedData = deriveSomeData(largeData);
    
    const children = useMemo(() => {
        return <div>{derivedData.someText}</div>
    }, [derivedData]);
}

Looks like useMemo can skip rendering wrapped in it, and the final update(by v-dom diff), but can't skip render function itself.

Personally,

I agree with using second argument as a selector with shallowEqual, since observedBits is a less common use case and selector can do what observedBits can do.
Also some times the needed data is a combination of nested context value and props in view hierachy, especially in normalized data structure with a nested view, when we want only to check the result of map[key], instead of the reference of map or the value of key, passing a selector can be very convenient:

function createSelectorWithProps(props) {
   return state => [state.map[props._id]];
}

function useContextWithProps(props) {
   return useContext(MyContext, createSelectorWithProps(props));
}

function ContextUsingComponent(props) {
    const [item] = useContextWithProps(props);
    
    // return ...........
}

But how to handle using multiple context?

 function ContextUsingComponent(props) {
     const [item] = useContextsWithProps(props, context1, context2, context3);
     
     // return ...........
 }

Finally the problem focuses on [rerender after calculate the data].
Thus I thought we need to useState with useObservable.
Observables trigger calculation, and shallowEqual the result, then set the result to local state.
Just the same as react-redux is doing, but with a hooks api style.

@FredyC

This comment has been minimized.

Copy link

FredyC commented Nov 20, 2018

I just found out about observedBits only thanks to @markerikson and somehow it felt like an awful solution. Working with bitmasks in JavaScript is not exactly something you see every day and imo devs are not really used to that concept much.

Besides, I find it rather awkward that I would need to kinda declare up front what my consumers might be interested in. What if there is a really an object (a.k.a store) with many properties and I would need each property to assign a bit number and most likely export some kind of map so a consumer can put a final bitmask together.

Well, in the end since I am a happy user of MobX, I don't care about this issue that much. Having an observable in the context, I can optimize rendering based on changes in that observable without any extra hassle of comparing stuff or having specific selectors. React won't probably introduce such a concept, but it could be one of the recommendations.

@TotooriaHyperion

This comment has been minimized.

Copy link

TotooriaHyperion commented Nov 22, 2018

how about this

// assuming createStore return a observable with some handler function or dispatch
function createMyContext() {
  const Context = React.createContext();
  function MyProvider({ children }) {
    const [store, setState] = useState(() => createStore());
    return <Context.Provider value={store}>{children}</Context.Provider>
  }
  return {
     Provider: MyProvider,
     Consumer: Context.Consumer,
     Context,
  }
}

const context1 = createMyContext();
const context2 = createMyContext();

 function calculateData(store1, store2) {
    //return something
 }

function ContextUsingComponent() {
  const store1 = useContext(context1.Context);
  const store2 = useContext(context2.Context);
  const [calculated, setCalculated] = useState(() => calculateData(store1, store2));
  function handleChange() {
     const next = calculateData(store1, store2);
     if (!shallowEqual(next, calculated)) {
        setCalculated(next);
     }
  }
  useEffect(() => {
     const sub1 = store1.subscribe(handleChange);
     const sub2 = store2.subscribe(handleChange);
     return () => {
        sub1.unsubscribe();
        sub2.unsubscribe();
     }
  }, [store1, store2])

  // use calculated to render something.
  // use store1.dispatch/store1.doSomething to update
}
@vijayst

This comment has been minimized.

Copy link

vijayst commented Nov 23, 2018

Without React NOT providing an option to cancel updates especially from context changes and because using useReducer hook causes various design constraints, we have to resort to good old Redux. Wrote an article based on developing a Redux clone based on existing API - context and hooks which explains more.

Two things are clear.

  1. Can't use Context API for global state unless we create a wrapper component (HOC, defeating purpose of hooks)
  2. No way to share state across container components when we use useReducer hook. Very rare that an app has independent container components for useReducer hook to be effective.
@TotooriaHyperion

This comment has been minimized.

Copy link

TotooriaHyperion commented Nov 23, 2018

@vijayst
I think this shouldn't be implemented as a "cancel" operation, it should be implemented as a way of "notice" instead.

Finally the problem focuses on [check if we need rerender after calculate the data].

This is exactly what react-redux do. And I wrote a example above to implement a react-redux like mechanism.
So let context to provide an observable is the convenient way to solve this problem. Rather than to "cancel" the update when we are already updating.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Dec 18, 2018

Yeah, I think I went into that conversation wanting to hear certain things, and the discussion spiraled off in directions I wasn't anticipating. (I also generally had way too much stuff running around in my head and not enough sleep, so the finer nuances of the discussion probably didn't sink in very well.)

@acdlite : fwiw, I definitely understand that more stuff is needed for concurrent support, per our whiteboard discussion earlier this year. My goal for React-Redux v6 was that it would at least be a step towards compatibility, not that it would actually be the "solution".

@KidkArolis

This comment has been minimized.

Copy link

KidkArolis commented Dec 18, 2018

@vijayst @sebmarkbage yes, unstable_batchedUpdates is a good solution for out of order and double rendering issues. I will definitely have a closer look at that.

But just wanted to make sure that the point gets across that in that implementation above, those issues do exist. If you dispatch 3 updates to the store, the app rerenders 3 times and the children rerender 6 times, and the children rerender before parents.

Of course, it depends on a few things: a) the store implementation could counter these (e.g. by utilising unstable_batchedUpdates internally and b) you won't experience those issues if you're updating the store in a user event callback, since React wraps event callbacks in unstable_batchedUpdates or equivalent.

And just for anyone following along.. Here's a demonstration: https://codesandbox.io/s/20w7l6zo3y.

@gnapse

This comment has been minimized.

Copy link

gnapse commented Dec 27, 2018

And what about this redux-react-hook from facebook itself? A quick glance at the README makes it look like they've achieved it* in combination with requiring users to useCallback in a couple of places.

* By "it" I mean using context with a partial selection of state, and preventing re-render if that partial state has not changed, even if other parts have changed. It is not a "bailing out" mechanism, but still.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Dec 27, 2018

@gnapse : looking at the code, that third-party hook appears to pass the store itself down via context, and subscribes to the store directly. That is equivalent to how React-Redux v5 works. The discussion in this thread is how to allow hooks and behavior that match the way React-Redux v6, Unstated, Formik, and other similar libraries work: passing the entire state object down via context. (Also, note that that's "just" from a Facebook employee, not specifically a Facebook-sponsored library, and that Facebook does not own or work on Redux directly.)

@gnapse

This comment has been minimized.

Copy link

gnapse commented Dec 27, 2018

Yes, sorry. I know it’s not an official Facebook solution and my way of putting it made it seem so.

Also, at the risk to continue to take this thread for stuff that’s not specifically about bailing out: is there something wrong with this approach? Maybe it’ll be problematic with concurrent react and suspense? If not, what’s the problem with continuing down that path?

@markerikson

This comment has been minimized.

Copy link

markerikson commented Dec 27, 2018

@gnapse : yes, as has been discussed multiple times in this thread already, external mutative stores and subscriptions are likely to not work correctly under concurrent React and Suspense.

@ccmartell

This comment has been minimized.

Copy link

ccmartell commented Jan 4, 2019

Semi-new to react and came across this problem as well. The useContext is re-rendering our whole app on every change.

Any chance to create something similar to the connect HOC from redux?
Something like:

import React, {useContext} from 'react'
import contextName from 'contextLocation'

const Component = ({ contextVariable }) => {
     return (
          <div>{contextVariable.b}</div>
     )
}

export default useContext(contextName, contextVariable, fn)(Component)

Where fn is the should component update function.

This way it might be able to stop the re-render if the function comes back false?
You may also be able to subscribe to multiple contexts this way by turning the first two variables into arrays or adding another HOC.

@dantman

This comment has been minimized.

Copy link
Contributor

dantman commented Jan 4, 2019

@ccmartell That's not really a hook, that's really just a HOC, so I wouldn't use use in the name.

With or without hooks you should be able to do that using recompose:

export default fromRenderProps(TheContext.Consumer, ({ contextVariable }) => ({ contextVariable }))(Component);

Then there's with-context and perhaps some other render prop helpers.

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 5, 2019

const themeContext = React.createContext({color, size, opacity})

function App({msg}) {
  const {color, size} = useContext(themeContext, ({color, size}) => ({color, size}))

  useOptimize((nextProp, nextState, nextContext) => {
    return nextProp.msg === msg && nextContext.color === color && nextContext.size === size
  })

  /* ... */
}

useOptimize(isEqual = shallowEqual)

example:
<App msg={'hello'} />
status of App: {props: {msg: 'hello'}, state: undefined, context: {color, size}}

@ignatiusreza

This comment has been minimized.

Copy link

ignatiusreza commented Jan 7, 2019

Whatever the solution is to the multiple roots problem might add further constraints on this that we don't even know yet.

Naively you might think that you can just add that on top but that doesn't actually work without breaking other features.

@sebmarkbage this might be what you've hinted in with being a "naive" solution.. but, just to put it on the table..

we already have a solution for rendering into multiple physical dom roots while still keeping a single virtual root using portal.. so, I wonder if the multiple roots problem can be solved by react automatically render a single shared root whose sole responsibility is to hold the state and render each "mount point" using portal inside a context provider..

@sebmarkbage

This comment has been minimized.

Copy link
Member

sebmarkbage commented Jan 7, 2019

@ignatiusreza That works for some cases but it only works with one React renderer/version at a time and don't play nicely with non-react code. It also doesn't let you coordinate the updates to individual roots with other content on the page - like the createBatch API in createRoot lets you do.

@ignatiusreza

This comment has been minimized.

Copy link

ignatiusreza commented Jan 7, 2019

@sebmarkbage thanks for the insight! if you don't mind, I still have some clarification I want to ask 🙇

and don't play nicely with non-react code.

sorry, i'm not sure why it doesn't play nicely with non-react code.. do you mind to elaborate a little?

It also doesn't let you coordinate the updates to individual roots with other content on the page

doesn't it means that portal also suffer from the same issue, which we potentially need to fix?

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 8, 2019

@sebmarkbage Why not try Proxy to optimize performance? seems it can work well.

const {color, size} = useContext(themeContext)

themeContext's value is wraped by Proxy, so we can know color and size are get in this component, we can use shallowEqual to optimize by default.

new Proxy(themeContext.value, {
  get() {/* ... */}
})

If color and size were not changed, React could skip render this component.

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 8, 2019

@sebmarkbage
Even more, we can use Proxy to do some amazing things.

function magicObject(parentObject, key, object) {
  return new Proxy(object, {
    set(target, prop, newValue) {
      if (newValue !== target[prop]) {
        parentObject[key] = magicObject(parentObject, key, object)
      }
      target[prop] = newValue
      return newValue
    }
  })
}
var a = {b: {c: 1}}
a.b = magicObject(a, 'b', a.b)

var b1 = a.b
a.b.c = 100
var b2 = a.b

console.log('b1 === b2', b1 === b2) // print `false`

It can work with shallowEqual.
And it can optimize performance for large array when shallow equal.

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 8, 2019

@gaearon @sebmarkbage
Work with Context:

context.value = {array: []}
magicAnObject(context)

then, context.value will be replaced with new Proxy as array was changed such as array.push(1) every time.
It is very friendly for shallowEqual.

Follow this rule, we can always use shallowEqual for optimizing performance without writing custom shouldUpdate.

@kingdaro

This comment has been minimized.

Copy link

kingdaro commented Jan 8, 2019

Proxies can't be reliably polyfilled to work on IE9 (React's target browser)

@pelotom

This comment has been minimized.

Copy link

pelotom commented Jan 8, 2019

Or even IE11.

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 11, 2019

I think we can do not optimize for ie user. React can still work properly without proxy, just without optimization.

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 11, 2019

Once React use Proxy API, needn't use shallowEqual anymore, dirtyValueKeys = new Set() is enough.
Proxy is for optimizing app automatically, provide an approach to optimize app manually if necessary(such as compatible with IE browser), but encourage users to use MVVM to optimize performance automatically.

@rabbitooops

This comment has been minimized.

Copy link

rabbitooops commented Jan 11, 2019

const {color, size} = useContext(themeContext) // Use Proxy API to optimize automatically
const {color, size} = useContext(themeContext, ['color', 'size']) // Compatible with IE
@pfgray

This comment has been minimized.

Copy link

pfgray commented Jan 14, 2019

I think there's two approaches that have been mentioned to this problem:


  1. A general purpose hook that throws to stop rendering.
useShouldComponentUpdate(prev => {
  return prev === value
}, value)

Pros: easy to understand, works for any hook, can make decisions based on values from many hooks
Cons: Can't "prevent" a render from starting, i.e. must start rendering to see if we should proceed


  1. Adding semantics to each hook where we think we’ll need the ability to stop re-renders.

For withState, this looks like: #14569. And we have yet to figure out what this looks like for useContext.

One thing that worries me about this approach is it forces clients to maintain references in order to establish equality.


TBH, I think there's room for both the changes @acdlite is proposing in #14569 and a generalized useShouldComponentUpdate

@acdlite

This comment has been minimized.

Copy link
Member

acdlite commented Jan 15, 2019

@pfgray

A general purpose hook that throws to stop rendering.

I don't see any possible semantic for useShouldComponentUpdate that wouldn't break composition. For example, if I'm writing a reusable custom hook called useFoo, I can't use useShouldComponentUpdate inside it without blocking updates from other hooks in the same component.

Similarly, if I'm writing a component and I include useShouldComponentUpdate at the top level, I can't use any custom hooks because I don't know what internal states or contexts it might include.

Adding semantics to each hook where we think we’ll need the ability to stop re-renders.

You say that as if there's an unbound number of primitive hooks that are a source of state, but there's actually only two: useReducer and useContext. (useState is built on top of useReducer so we get that one for "free.") The PR you linked to addresses both.

I also think you have the mental model backwards. It's not about stopping the render; it's about telling React if it can't stop the render. Every time something changes, we dirty a bit. If we get to the end of the component and the bit is still clean, then we know we can bail out. If anything changes, we can't bail out. No single hook has the ability to prevent the component from rendering, because that would be anti-modular, as discussed above.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Jan 15, 2019

@acdlite : Perhaps I misread that PR when I first glanced at it, because it didn't seem like it dealt with context at all. Were context-related changes added later, or did I just miss something the first time?

If it really does handle useContext in some way, what actual capabilities does it add?

Updating useReducer() is obvious, because you're adding the ability to bail out of the previous state value is returned as-is. Per Sebastian's comments earlier in this thread, there's not currently something equivalent for useContext().

@acdlite

This comment has been minimized.

Copy link
Member

acdlite commented Jan 15, 2019

Should have been clearer: it accounts for context changes before deciding to bail out. It doesn't add a new way to bail out of context. We have some ideas there but we haven't made a final decision yet.

@markerikson

This comment has been minimized.

Copy link

markerikson commented Jan 15, 2019

In other words, "if I return the previous state from my reducer, but there's also a context change, it will complete the update". Got it.

Definitely looking forward to whatever you come up with on the useContext side. Happy to give feedback when you've got something to discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment