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

Page data HMR #3132

Merged

Conversation

alexkirsz
Copy link
Contributor

@alexkirsz alexkirsz commented Dec 23, 2022

Based on #2968

This builds upon #2968 and @ForsakenHarmony's work on data routes to enable page data HMR.

Page data HMR is a bit more clever than it is in Next.js as we won't re-render a Node.js result for each page file update. Instead, thanks to the StripPageDefaultExport transform, there are three versions of the page chunks:

  • client-side (strips page data exports);
  • server-side (full);
  • data server-side (strips page default export).

Instead of subscribing to the full server-side result, on hydration, the client-side page separately subscribes to:

  • client-side updates (already the case);
  • data server-side updates (new).

This means that updating something that only affects the page component will only cause a client-side update and no Node.js re-rendering, while updating something that only affects the data will only cause a server-side update.

I'm marking this as a draft for now as there are still a few areas to test/investigate:

  • When something that is used in both the default page export and data exports is changed, this will cause two HMR updates: one data update, and one client-side chunk update. The same case breaks in Next.js, where we will receive a client-side update, but no server-side update, ending up with an incorrect result.
  • Differences between getStaticProps/getServerSideProps, as well as getInitialProps (need to talk with @timneutkens about this) (see Exclude development from static html/json render on data request next.js#44523)

@vercel
Copy link

vercel bot commented Dec 23, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

10 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jan 4, 2023 at 1:35PM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Jan 4, 2023 at 1:35PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jan 4, 2023 at 1:35PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jan 4, 2023 at 1:35PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Jan 4, 2023 at 1:35PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Jan 4, 2023 at 1:35PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Jan 4, 2023 at 1:35PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jan 4, 2023 at 1:35PM (UTC)
turbo-site ⬜️ Ignored (Inspect) Visit Preview Jan 4, 2023 at 1:35PM (UTC)
turbo-vite-web ⬜️ Ignored (Inspect) Jan 4, 2023 at 1:35PM (UTC)

*/
function subscribePageData({ assetPrefix }: { assetPrefix: string }) {
const pageChunkPath = location.pathname.slice(1);
const dataPath = `_next/data/development/${pageChunkPath}.json`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to update this on client side navigation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I've updated it so it unsubscribes and resubscribes when navigating to a different page.

Base automatically changed from alexkirsz/web-273-invert-next-ssg to main January 3, 2023 10:42
@alexkirsz alexkirsz force-pushed the alexkirsz/web-218-hmr-for-data-in-pages-getserversideprops branch from 333d500 to b80c5c2 Compare January 3, 2023 11:13
@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Rust tests

See workflow summary for details

@alexkirsz alexkirsz marked this pull request as ready for review January 3, 2023 13:19
@alexkirsz alexkirsz requested a review from a team as a code owner January 3, 2023 13:19
@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023

Benchmark for 23cd37a

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8047.28µs ± 73.56µs 8072.05µs ± 59.62µs +0.31%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8302.11µs ± 39.79µs 8321.16µs ± 61.51µs +0.23%
bench_hmr_to_commit/Turbopack RSC/1000 modules 463.07ms ± 2.18ms 458.99ms ± 1.44ms -0.88%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8130.98µs ± 49.85µs 8207.53µs ± 61.64µs +0.94%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7128.02µs ± 54.84µs 7143.25µs ± 31.30µs +0.21%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7301.74µs ± 20.67µs 7281.99µs ± 44.08µs -0.27%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7237.69µs ± 47.10µs 7292.86µs ± 66.64µs +0.76%
bench_hydration/Turbopack RCC/1000 modules 3267.75ms ± 9.04ms 3281.98ms ± 15.45ms +0.44%
bench_hydration/Turbopack RSC/1000 modules 2755.59ms ± 7.91ms 2738.08ms ± 6.81ms -0.64%
bench_hydration/Turbopack SSR/1000 modules 2593.96ms ± 5.65ms 2588.56ms ± 4.47ms -0.21%
bench_startup/Turbopack CSR/1000 modules 1614.82ms ± 6.38ms 1601.41ms ± 4.92ms -0.83%
bench_startup/Turbopack RCC/1000 modules 2465.15ms ± 4.50ms 2454.94ms ± 8.17ms -0.41%
bench_startup/Turbopack RSC/1000 modules 2353.75ms ± 5.56ms 2351.43ms ± 5.06ms -0.10%
bench_startup/Turbopack SSR/1000 modules 2013.79ms ± 3.22ms 2020.86ms ± 4.30ms +0.35%

@alexkirsz alexkirsz force-pushed the alexkirsz/web-218-hmr-for-data-in-pages-getserversideprops branch from 628f0a5 to 680803d Compare January 3, 2023 16:32
@@ -249,8 +261,11 @@ function handleSocketMessage(msg: ServerMessage) {
}
}

export function onChunkUpdate(chunkPath: ChunkPath, callback: UpdateCallback) {
onUpdate(
export function onChunkUpdate(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have consistent naming here, other functions above and below start with subscribeTo, this one with on

@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023

Benchmark for e8f5344

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8564.60µs ± 41.41µs 8592.76µs ± 41.08µs +0.33%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8857.85µs ± 56.79µs 8868.44µs ± 60.89µs +0.12%
bench_hmr_to_commit/Turbopack RSC/1000 modules 483.68ms ± 1.50ms 479.20ms ± 3.02ms -0.93%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8660.42µs ± 63.95µs 8682.74µs ± 106.97µs +0.26%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7583.69µs ± 37.60µs 7588.40µs ± 62.35µs +0.06%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7657.26µs ± 51.40µs 7649.42µs ± 51.54µs -0.10%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7598.74µs ± 68.50µs 7622.69µs ± 71.70µs +0.32%
bench_hydration/Turbopack RCC/1000 modules 3405.78ms ± 7.61ms 3384.76ms ± 14.52ms -0.62%
bench_hydration/Turbopack RSC/1000 modules 2860.40ms ± 7.88ms 2859.92ms ± 7.71ms -0.02%
bench_hydration/Turbopack SSR/1000 modules 2665.26ms ± 10.50ms 2694.89ms ± 5.98ms +1.11%
bench_startup/Turbopack CSR/1000 modules 1664.39ms ± 5.22ms 1663.19ms ± 5.77ms -0.07%
bench_startup/Turbopack RCC/1000 modules 2575.29ms ± 10.10ms 2541.87ms ± 7.25ms -1.30%
bench_startup/Turbopack RSC/1000 modules 2431.25ms ± 7.24ms 2436.40ms ± 9.70ms +0.21%
bench_startup/Turbopack SSR/1000 modules 2083.98ms ± 4.60ms 2083.53ms ± 5.97ms -0.02%

@alexkirsz alexkirsz added the pr: automerge Kodiak will merge these automatically after checks pass label Jan 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2023

Benchmark for a69f100

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 7957.00µs ± 76.33µs 7983.15µs ± 48.80µs +0.33%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8243.36µs ± 66.13µs 8297.25µs ± 44.91µs +0.65%
bench_hmr_to_commit/Turbopack RSC/1000 modules 464.37ms ± 2.27ms 468.81ms ± 3.09ms +0.96%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8077.46µs ± 52.80µs 8078.81µs ± 70.68µs +0.02%
bench_hmr_to_eval/Turbopack CSR/1000 modules 7094.76µs ± 50.93µs 7000.44µs ± 46.29µs -1.33%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7173.37µs ± 56.77µs 7163.73µs ± 57.49µs -0.13%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7092.95µs ± 60.86µs 7127.35µs ± 49.94µs +0.48%
bench_hydration/Turbopack RCC/1000 modules 3321.95ms ± 10.10ms 3309.11ms ± 8.70ms -0.39%
bench_hydration/Turbopack RSC/1000 modules 2804.08ms ± 8.07ms 2812.02ms ± 9.34ms +0.28%
bench_hydration/Turbopack SSR/1000 modules 2611.18ms ± 7.51ms 2618.63ms ± 13.24ms +0.29%
bench_startup/Turbopack CSR/1000 modules 1624.19ms ± 6.45ms 1621.90ms ± 5.88ms -0.14%
bench_startup/Turbopack RCC/1000 modules 2494.17ms ± 5.53ms 2500.73ms ± 7.94ms +0.26%
bench_startup/Turbopack RSC/1000 modules 2391.97ms ± 9.51ms 2372.27ms ± 5.83ms -0.82%
bench_startup/Turbopack SSR/1000 modules 2043.35ms ± 4.72ms 2039.15ms ± 4.99ms -0.21%

@alexkirsz alexkirsz merged commit 8d16580 into main Jan 4, 2023
@alexkirsz alexkirsz deleted the alexkirsz/web-218-hmr-for-data-in-pages-getserversideprops branch January 4, 2023 15:54
jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
Based on vercel/turbo#2968 

This builds upon vercel/turbo#2968 and @ForsakenHarmony's work on data routes to
enable page data HMR.

Page data HMR is a bit more clever than it is in Next.js as we won't
re-render a Node.js result for each page file update. Instead, thanks to
the `StripPageDefaultExport` transform, there are three versions of the
page chunks:
* client-side (strips page data exports);
* server-side (full);
* data server-side (strips page default export).

Instead of subscribing to the full server-side result, on hydration, the
client-side page separately subscribes to:
* client-side updates (already the case);
* data server-side updates (new).

This means that updating something that only affects the page component
will only cause a client-side update and **no Node.js re-rendering**,
while updating something that only affects the data will only cause a
server-side update.

~~I'm marking this as a draft for now as there are still a few areas to
test/investigate:~~
- [x] When something that is used in both the default page export and
data exports is changed, this will cause *two* HMR updates: one data
update, and one client-side chunk update. **The same case breaks in
Next.js, where we will receive a client-side update, but no server-side
update, ending up with an incorrect result.**
- [x] Differences between `getStaticProps/getServerSideProps`, as well
as `getInitialProps` (need to talk with @timneutkens about this) (see
#44523)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
Based on vercel/turbo#2968 

This builds upon vercel/turbo#2968 and @ForsakenHarmony's work on data routes to
enable page data HMR.

Page data HMR is a bit more clever than it is in Next.js as we won't
re-render a Node.js result for each page file update. Instead, thanks to
the `StripPageDefaultExport` transform, there are three versions of the
page chunks:
* client-side (strips page data exports);
* server-side (full);
* data server-side (strips page default export).

Instead of subscribing to the full server-side result, on hydration, the
client-side page separately subscribes to:
* client-side updates (already the case);
* data server-side updates (new).

This means that updating something that only affects the page component
will only cause a client-side update and **no Node.js re-rendering**,
while updating something that only affects the data will only cause a
server-side update.

~~I'm marking this as a draft for now as there are still a few areas to
test/investigate:~~
- [x] When something that is used in both the default page export and
data exports is changed, this will cause *two* HMR updates: one data
update, and one client-side chunk update. **The same case breaks in
Next.js, where we will receive a client-side update, but no server-side
update, ending up with an incorrect result.**
- [x] Differences between `getStaticProps/getServerSideProps`, as well
as `getInitialProps` (need to talk with @timneutkens about this) (see
#44523)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants