[Experimental] Add useInsertionEffect #21913
Merged
+862
−6
Conversation
|
Comparing: 81db4eb...ab6ad9f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
I’d say effect lists were the usual cause of bugs so you have a higher chance of doing it right with the current approach! |
rickhanlonii
force-pushed
the
rh-usemutationeffect
branch
9 times, most recently
from
a10210c
to
c6ceff6
acdlite
reviewed
Jul 20, 2021
Looks good overall! Let's discuss the timing of the destroy function before merging
ryota-murakami
reviewed
Jul 24, 2021
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
Show resolved
Hide resolved
|
@rickhanlonii just curious is the point of having this hook is to handle something even earlier than |
rickhanlonii
changed the title
|
@windmaomao I updated the description with the use case |
rickhanlonii
force-pushed
the
rh-usemutationeffect
branch
4 times, most recently
from
bac179d
to
36cbc67
acdlite
reviewed
Sep 10, 2021
salazarm
reviewed
Sep 13, 2021
acdlite
reviewed
Sep 13, 2021
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
Outdated
Show resolved
Hide resolved
acdlite
approved these changes
Sep 13, 2021
Looks good!
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
Show resolved
Hide resolved
gaearon
reviewed
Sep 13, 2021
| @@ -191,6 +192,18 @@ function useLayoutEffect( | |||
| }); | |||
| } | |||
|
|
|||
| function useInsertionEffect( | |||
Didn't we decide to drop the ion and just do useInsertEffect? timberlake.gif
There is a legit noun.
Yes, and then no. useInsertEffect sounds too much like the thing you're inserting is an effect, which I think we noticed when we started talking about it again and thought of it as a verb first (which is probably the natural reading of it).
Andarist
reviewed
Sep 14, 2021
| function useInsertionEffect( | ||
| create: () => mixed, | ||
| inputs: Array<mixed> | void | null, | ||
| ) { | ||
| if (__DEV__) { | ||
| currentHookNameInDev = 'useInsertionEffect'; | ||
| console.error( | ||
| 'useInsertionEffect does nothing on the server, because its effect cannot ' + | ||
| "be encoded into the server renderer's output format. This will lead " + | ||
| 'to a mismatch between the initial, non-hydrated UI and the intended ' + | ||
| 'UI. To avoid this, useInsertionEffect should only be used in ' + | ||
| 'components that render exclusively on the client.', | ||
| ); | ||
| } | ||
| } |
How are CSS-in-JS libraries supposed to handle SSR then? If the main use for this hook is stylesheet libraries then I think we need to document a compelling strategy for how those should be implemented "end to end".
In Emotion we currently conditionally render <style/> tags "inline" in the SSRed output and expect our hydration function to move them to <head/> (most commonly) before React "sees" them - it's tricky, but it sort of works (at least right now). Is this an acceptable approach?
Especially that last part of this error message seems to be off - given the primary use case for this hook. Style-based components cannot be exclusively rendered on the client because that would almost completely obliviate the sense of SSRing applications using those kinds of libraries.
@Andarist short term they'll insert them into the stream separately, chunk points. @sebmarkbage is writing up a tutorial for how to do that as a working group post. Long term, we have an RFC we're going to publish for how we intend to handle this in an integrated way that works isomorphically.
Interesting, thanks for the comment and I can't wait to read how this will work
However, that probably means that the last paragraph of my comment is still valid - the error message here seems to be somewhat inaccurate and could throw somebody off. Unless you expect library maintainers to ignore that and "know better".
It's probably wrong in the same sense as the useLayoutEffect warning being wrong since it can also be done differently on the server to inject through different means and needs to be done conditionally to workaround these issues.
Thanks @rickhanlonii, now i understand the usage. Basically similar to a layoutEffect, but applied right before the mutation finishes. so the order is more like insertEffect -> mutationEffect -> layoutEffect -> ... -> passiveEffect. |
facebook-github-bot
added a commit
to facebook/react-native
that referenced
this issue
Sep 22, 2021
Summary: This sync includes the following changes: - **[f4ac680c7](facebook/react@f4ac680 )**: Fixed broken build script --unsafe-partial flag ([#22324](facebook/react#22324)) //<Brian Vaughn>// - **[67222f044](facebook/react@67222f0 )**: [Experiment] Warn if callback ref returns a function ([#22313](facebook/react#22313)) //<Dan Abramov>// - **[263cfa6ec](facebook/react@263cfa6 )**: [Experimental] Add useInsertionEffect ([#21913](facebook/react#21913)) //<Ricky>// - **[806aaa2e2](facebook/react@806aaa2 )**: [useSES shim] Import prefixed native API ([#22310](facebook/react#22310)) //<Andrew Clark>// - **[fd5e01c2e](facebook/react@fd5e01c )**: [useSES/extra] Reuse old selection if possible ([#22307](facebook/react#22307)) //<Andrew Clark>// - **[33226fada](facebook/react@33226fa )**: Check for store mutations before commit ([#22290](facebook/react#22290)) //<Andrew Clark>// - **[86c7ca70a](facebook/react@86c7ca7 )**: Fix link ([#22296](facebook/react#22296)) //<Konstantin Popov>// - **[0fd195f29](facebook/react@0fd195f )**: update error message to include useLayoutEffect or useEffect on bad e… ([#22279](facebook/react#22279)) //<salazarm>// - **[8f96c6b2a](facebook/react@8f96c6b )**: [Bugfix] Prevent infinite update loop caused by a synchronous update in a passive effect ([#22277](facebook/react#22277)) //<Andrew Clark>// - **[4ce89a58d](facebook/react@4ce89a5 )**: Test bad useEffect return value with noop-renderer ([#22258](facebook/react#22258)) //<Sebastian Silbermann>// - **[a3fde2358](facebook/react@a3fde23 )**: Detect subscriptions wrapped in startTransition ([#22271](facebook/react#22271)) //<salazarm>// Changelog: [General][Changed] - React Native sync for revisions 95d762e...e8feb11 jest_e2e[run_all_tests] Reviewed By: rickhanlonii Differential Revision: D30966369 fbshipit-source-id: 6c88e591005deb1fd93493628ef4695add49186c
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Overview
Adds back the experimental
useMutationEffecthook asuseInsertionEffect, initially intended for stylesheet libraries.Semantics
This hook is called right before mutations are made to the host, to allow inserting dependencies of the soon-to-be mutated host nodes. The canonical example use case is for inserting styles into the DOM so that they're available before reading layout information from DOM nodes that they apply to (like reading height in
useLayoutEffect). Since this hook is limited in scope, this hook does not have access to refs and cannot schedule updates.The lifecycle of this hook is a little different than other hooks.
useInsertionEffectwill interleave create and destroy while traversing the tree in the beforeMutation phase. This means that as we traverse the tree before mutations (e.g. the same time we callgetSnapshotBeforeUpdate), and for each component we will destroy all of the insertion effects and then create all of the insertion effects for that component.The resulting in ordering is something like:
Contrast this with layout effects, which do a two passes. One to destroy all of the layout effects in every component, and a second to create them:
The text was updated successfully, but these errors were encountered: