-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Suspended components should rerender through sCU #2125
Conversation
Your current approach is currently running into similar issues as #2159 This makes me rethink our decision to unmount suspended content instead of adding |
db8b819
to
399ca4a
Compare
Size Change: +122 B (0%) Total Size: 38.4 kB
ℹ️ View Unchanged
|
399ca4a
to
658b40b
Compare
c._vnode._children[0] = c.state._suspended; | ||
c.setState({ _suspended: (c._detachOnNextRender = null) }); | ||
|
||
let suspended; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't spend much time golfing this so there might be a smaller alternative
|
||
let suspended; | ||
while ((suspended = c._suspenders.pop())) { | ||
suspended.forceUpdate(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I think this is a no-op if no sCU component is between Suspense and Lazy cuz process()
will sort the render such that Suspense and all its children are diffed first, then when the Lazy forceUpdate are handled, the component will no longer be marked "_dirty" so they will be skipped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for always commenting on your own PR's it actually really helps out a lot in these cases! Awesome work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
@@ -55,7 +56,7 @@ export function createSuspender(DefaultComponent) { | |||
} | |||
|
|||
render(props, state) { | |||
return state.Lazy ? h(state.Lazy, {}) : h(DefaultComponent, {}); | |||
return state.Lazy ? h(state.Lazy, props) : h(DefaultComponent, props); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are props really meant to be passed down?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my intention. Mimics what React.lazy allows: https://codesandbox.io/s/reactlazy-prop-passing-b8txb
* Add test for suspending component through sCU blocking component * Add test for passing props through Lazy * Add test for using suspense with createContext * Add tests for initially lazy and delayed lazy components with sCU and createContext * Fix typo in unmounted component warning * Add test for multi-suspend under sCU * Improve debug suspense tests * Force update all suspended components when suspension is complete (+31 B)
Resurrecting the awesome work @sventschui did in #1660 now built on top of @jviide and @prateekbh suspense changes. Just calls
forceUpdate
on the suspended component to guarantee it rerenders when the promise resolves.Fixes #2176