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

[Fizz] Expose callbacks in options for when various stages of the content is done #21056

Merged
merged 4 commits into from
Mar 23, 2021

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Mar 23, 2021

onError is called when an error happens anywhere in the tree. It can be called multiple times if it's not a fatal error. This can be used for logging or setting a status code of a request.

onCompleteAll is called when all pending work is done but it may not have flushed yet. If you don't want to incrementally stream the content, you can wait to call startWriting until this point. That way we only emit the final HTML.

onReadyToStream is called when there is at least a root fallback ready to show. You typically don't need this callback because you can just startWriting immediately if you don't expect to need to change the status code or anything.

Another strategy for deciding when to call startWriting is just a timer based approach that starts writing after some timeout.

In the future we can also add callbacks for when certain priority levels are complete. So you can for example wait until all the high pri content is done.

@sebmarkbage sebmarkbage requested a review from gaearon March 23, 2021 03:34
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 23, 2021
@sizebot
Copy link

sizebot commented Mar 23, 2021

Comparing: 25bfa28...b28626f

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 122.80 kB 122.80 kB = 39.50 kB 39.50 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.21 kB 129.21 kB = 41.50 kB 41.50 kB
facebook-www/ReactDOM-prod.classic.js = 408.57 kB 408.57 kB = 75.63 kB 75.63 kB
facebook-www/ReactDOM-prod.modern.js = 396.83 kB 396.83 kB = 73.70 kB 73.70 kB
facebook-www/ReactDOMForked-prod.classic.js = 408.58 kB 408.58 kB = 75.63 kB 75.63 kB
oss-experimental/react-server/cjs/react-server.production.min.js +5.30% 8.43 kB 8.88 kB +3.11% 3.06 kB 3.15 kB
oss-stable/react-server/cjs/react-server.production.min.js +5.30% 8.43 kB 8.88 kB +3.11% 3.06 kB 3.15 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.browser.production.min.js +5.00% 10.84 kB 11.38 kB +3.23% 4.00 kB 4.13 kB
oss-experimental/react-dom/umd/react-dom-unstable-fizz.browser.production.min.js +4.89% 11.03 kB 11.57 kB +3.21% 4.08 kB 4.21 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.node.production.min.js +4.70% 11.05 kB 11.57 kB +3.00% 4.03 kB 4.15 kB
oss-experimental/react-server/cjs/react-server.development.js +3.54% 33.53 kB 34.72 kB +2.69% 8.86 kB 9.10 kB
oss-stable/react-server/cjs/react-server.development.js +3.54% 33.53 kB 34.72 kB +2.69% 8.86 kB 9.10 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.browser.development.js +3.39% 48.53 kB 50.17 kB +2.94% 12.51 kB 12.88 kB
oss-experimental/react-dom/umd/react-dom-unstable-fizz.browser.development.js +3.35% 51.06 kB 52.78 kB +2.94% 12.67 kB 13.04 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-server.production.min.js +2.85% 2.49 kB 2.56 kB +2.64% 0.99 kB 1.01 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-server.production.min.js +2.85% 2.49 kB 2.56 kB +2.64% 0.99 kB 1.01 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.node.development.js +2.70% 48.80 kB 50.12 kB +2.05% 12.52 kB 12.78 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-server.development.js +2.51% 5.21 kB 5.34 kB +1.80% 1.50 kB 1.53 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-server.development.js +2.51% 5.21 kB 5.34 kB +1.80% 1.50 kB 1.53 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-server/cjs/react-server.production.min.js +5.30% 8.43 kB 8.88 kB +3.11% 3.06 kB 3.15 kB
oss-stable/react-server/cjs/react-server.production.min.js +5.30% 8.43 kB 8.88 kB +3.11% 3.06 kB 3.15 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.browser.production.min.js +5.00% 10.84 kB 11.38 kB +3.23% 4.00 kB 4.13 kB
oss-experimental/react-dom/umd/react-dom-unstable-fizz.browser.production.min.js +4.89% 11.03 kB 11.57 kB +3.21% 4.08 kB 4.21 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.node.production.min.js +4.70% 11.05 kB 11.57 kB +3.00% 4.03 kB 4.15 kB
oss-experimental/react-server/cjs/react-server.development.js +3.54% 33.53 kB 34.72 kB +2.69% 8.86 kB 9.10 kB
oss-stable/react-server/cjs/react-server.development.js +3.54% 33.53 kB 34.72 kB +2.69% 8.86 kB 9.10 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.browser.development.js +3.39% 48.53 kB 50.17 kB +2.94% 12.51 kB 12.88 kB
oss-experimental/react-dom/umd/react-dom-unstable-fizz.browser.development.js +3.35% 51.06 kB 52.78 kB +2.94% 12.67 kB 13.04 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-server.production.min.js +2.85% 2.49 kB 2.56 kB +2.64% 0.99 kB 1.01 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-server.production.min.js +2.85% 2.49 kB 2.56 kB +2.64% 0.99 kB 1.01 kB
oss-experimental/react-dom/cjs/react-dom-unstable-fizz.node.development.js +2.70% 48.80 kB 50.12 kB +2.05% 12.52 kB 12.78 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-server.development.js +2.51% 5.21 kB 5.34 kB +1.80% 1.50 kB 1.53 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-server.development.js +2.51% 5.21 kB 5.34 kB +1.80% 1.50 kB 1.53 kB

Generated by 🚫 dangerJS against b28626f

This allows you to log errors or set things like status codes.
This is typically not needed because if you want to stream when the
root is ready you can just start writing immediately.
@sebmarkbage sebmarkbage force-pushed the fizzcallbacks branch 2 times, most recently from a2f0399 to c00f8ba Compare March 23, 2021 18:01
@sebmarkbage sebmarkbage merged commit 435cff9 into facebook:master Mar 23, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Apr 6, 2021
Summary:
This sync includes the following changes:
- **[c9aab1c9d](facebook/react@c9aab1c9d )**: react-refresh@0.10.0 //<Dan Abramov>//
- **[516b76b9a](facebook/react@516b76b9a )**: [Fast Refresh] Support callthrough HOCs ([#21104](facebook/react#21104)) //<Dan Abramov>//
- **[0853aab74](facebook/react@0853aab74 )**: Log all errors to console.error by default ([#21130](facebook/react#21130)) //<Sebastian Markbåge>//
- **[d1294c9d4](facebook/react@d1294c9d4 )**: Add global onError handler ([#21129](facebook/react#21129)) //<Sebastian Markbåge>//
- **[64983aab5](facebook/react@64983aab5 )**: Remove redundant setUpdatePriority call ([#21127](facebook/react#21127)) //<Andrew Clark>//
- **[634cc52e6](facebook/react@634cc52e6 )**: Delete dead variable: currentEventWipLanes ([#21123](facebook/react#21123)) //<Andrew Clark>//
- **[1102224bb](facebook/react@1102224bb )**: Fix: flushSync changes priority inside effect ([#21122](facebook/react#21122)) //<Andrew Clark>//
- **[dbe98a5aa](facebook/react@dbe98a5aa )**: Move sync task queue to its own module ([#21109](facebook/react#21109)) //<Andrew Clark>//
- **[3ba5c8737](facebook/react@3ba5c8737 )**: Remove Scheduler indirection ([#21107](facebook/react#21107)) //<Andrew Clark>//
- **[46b68eaf6](facebook/react@46b68eaf6 )**: Delete LanePriority type ([#21090](facebook/react#21090)) //<Andrew Clark>//
- **[dcd13045e](facebook/react@dcd13045e )**: Use Lane to track root callback priority ([#21089](facebook/react#21089)) //<Andrew Clark>//
- **[5f21a9fca](facebook/react@5f21a9fca )**: Clean up host pointers in level 2 of clean-up flag ([#21112](facebook/react#21112)) //<Andrew Clark>//
- **[32d6f39ed](facebook/react@32d6f39ed )**: [Fizz] Support special HTML/SVG/MathML tags to suspend ([#21113](facebook/react#21113)) //<Sebastian Markbåge>//
- **[a77dd13ed](facebook/react@a77dd13ed )**: Delete enableDiscreteEventFlushingChange ([#21110](facebook/react#21110)) //<Andrew Clark>//
- **[048ee4c0c](facebook/react@048ee4c0c )**: Use `act` in fuzz tester to flush expired work ([#21108](facebook/react#21108)) //<Andrew Clark>//
- **[556644e23](facebook/react@556644e23 )**: Fix plurals ([#21106](facebook/react#21106)) //<Sebastian Markbåge>//
- **[8b741437b](facebook/react@8b741437b )**: Rename SuspendedWork to Task ([#21105](facebook/react#21105)) //<Sebastian Markbåge>//
- **[38a1aedb4](facebook/react@38a1aedb4 )**: [Fizz] Add FormatContext and Refactor Work ([#21103](facebook/react#21103)) //<Sebastian Markbåge>//
- **[1b7e471b9](facebook/react@1b7e471b9 )**: React Native New Architecture: Support passing nativeViewTag to getInspectorDataForViewAtPoint callback, for React DevTools compat ([#21080](facebook/react#21080)) //<Joshua Gross>//
- **[4a99c5c3a](facebook/react@4a99c5c3a )**: Use highest priority lane to detect interruptions ([#21088](facebook/react#21088)) //<Andrew Clark>//
- **[77be52729](facebook/react@77be52729 )**: Remove LanePriority from computeExpirationTime ([#21087](facebook/react#21087)) //<Andrew Clark>//
- **[3221e8fba](facebook/react@3221e8fba )**: Remove LanePriority from getBumpedLaneForHydration ([#21086](facebook/react#21086)) //<Andrew Clark>//
- **[05ec0d764](facebook/react@05ec0d764 )**: Entangled expired lanes with SyncLane ([#21083](facebook/react#21083)) //<Andrew Clark>//
- **[03ede83d2](facebook/react@03ede83d2 )**: Use EventPriority to track update priority ([#21082](facebook/react#21082)) //<Andrew Clark>//
- **[a63f0953b](facebook/react@a63f0953b )**: Delete SyncBatchedLane ([#21061](facebook/react#21061)) //<Ricky>//
- **[fa868d6be](facebook/react@fa868d6be )**: Make opaque EventPriority type a Lane internally ([#21065](facebook/react#21065)) //<Andrew Clark>//
- **[eb58c3909](facebook/react@eb58c3909 )**: react-hooks/exhaustive-deps: Handle optional chained methods as dependency ([#20204](facebook/react#20204)) ([#20247](facebook/react#20247)) //<Ari Perkkiö>//
- **[7b84dbd16](facebook/react@7b84dbd16 )**: Fail build on deep requires in npm packages ([#21063](facebook/react#21063)) //<Dan Abramov>//
- **[2c9d8efc8](facebook/react@2c9d8efc8 )**: Add react-reconciler/constants entry point ([#21062](facebook/react#21062)) //<Dan Abramov>//
- **[d0eaf7829](facebook/react@d0eaf7829 )**: Move priorities to separate import to break cycle ([#21060](facebook/react#21060)) //<Andrew Clark>//
- **[435cff986](facebook/react@435cff986 )**: [Fizz] Expose callbacks in options for when various stages of the content is done ([#21056](facebook/react#21056)) //<Sebastian Markbåge>//
- **[25bfa287f](facebook/react@25bfa287f )**: [Experiment] Add feature flag for more aggressive memory clean-up of deleted fiber trees ([#21039](facebook/react#21039)) //<Benoit Girard>//
- **[8fe7810e7](facebook/react@8fe7810e7 )**: Remove already completed comment ([#21054](facebook/react#21054)) //<Sebastian Markbåge>//
- **[6c3202b1e](facebook/react@6c3202b1e )**: [Fizz] Use identifierPrefix to avoid conflicts within the same response ([#21037](facebook/react#21037)) //<Sebastian Markbåge>//
- **[dcdf8de7e](facebook/react@dcdf8de7e )**: Remove discrete lanes and priorities ([#21040](facebook/react#21040)) //<Andrew Clark>//
- **[ca99ae97b](facebook/react@ca99ae97b )**: Replace some flushExpired callsites ([#20975](facebook/react#20975)) //<Ricky>//
- **[1fafac002](facebook/react@1fafac002 )**: Use SyncLane for discrete event hydration ([#21038](facebook/react#21038)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 6d3ecb7...c9aab1c

jest_e2e[run_all_tests]

Reviewed By: JoshuaGross

Differential Revision: D27436763

fbshipit-source-id: da79a41e26bffdcdacd293178062edf098e9b58a
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
…tent is done (facebook#21056)

* Report errors to a global handler

This allows you to log errors or set things like status codes.

* Add complete callback

* onReadyToStream callback

This is typically not needed because if you want to stream when the
root is ready you can just start writing immediately.

* Rename onComplete -> onCompleteAll
@salazarm
Copy link
Contributor

salazarm commented Sep 14, 2021

So you can for example wait until all the high pri content is done.

Remind me what content is considered high priority in the context of server rendering? :)

@sebmarkbage
Copy link
Collaborator Author

I'm not actually sure what I meant in this context. Not sure it makes sense to wait for. :)

That said, there are a couple of different categories of priority. The ones implemented so far:

  • A client rendered or errored boundary that was already emitted, but we didn't know it would be client rendered, has higher priority because it's small and we need to signal to the client to get started on rendering it asap.
  • A fully completed boundary has higher priority than a partially completed one because the partially completed one won't be displayed anyway. We can just preemptively send it down. This

Things that we haven't done yet:

  • We could have an explicit API that determines relative priority between siblings Suspense boundaries. Some content on the page is more important than other.
  • <link rel="preload" /> tags used within one boundary have higher priority than all other boundaries so that they can start as early as possible.
  • <link rel="stylesheet" /> has higher priority than the content that they correspond to to avoid FOUC but may not be higher priority than other content.
  • <script> containing JSON data that will be used to hydrate content has lower priority than the content for that boundary itself since it's unnecessary if you can't see it yet. However, it might be higher than other content.
  • Inlined modules like <script> tags can be used to optimistically send down scripts inline to avoid issues with long latency but these could race against a <script src="..."> in case it's already cached or can be reached faster over a different wire. But these shouldn't take preference over actual content.
  • <link rel="prefetch" /> used to prefetch for future navigations have lower priority than everything else but can be emitted earlier if stalled.
  • JSON data used by future navigations (e.g. prerendering) has lower priority than anything else but can be done in the same request to avoid spinning up a new environment (although this technique goes against the economics of running on "the edge" with time limits).

@salazarm
Copy link
Contributor

Makes sense, thanks for the reply!

threepointone pushed a commit to threepointone/react that referenced this pull request Oct 24, 2021
…tent is done (facebook#21056)

* Report errors to a global handler

This allows you to log errors or set things like status codes.

* Add complete callback

* onReadyToStream callback

This is typically not needed because if you want to stream when the
root is ready you can just start writing immediately.

* Rename onComplete -> onCompleteAll
threepointone pushed a commit to threepointone/react that referenced this pull request Oct 24, 2021
…tent is done (facebook#21056)

* Report errors to a global handler

This allows you to log errors or set things like status codes.

* Add complete callback

* onReadyToStream callback

This is typically not needed because if you want to stream when the
root is ready you can just start writing immediately.

* Rename onComplete -> onCompleteAll
@gaearon gaearon mentioned this pull request Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants