Skip to content

[dynamicIO] cache tracking for import() #74152

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

Open
wants to merge 7 commits into
base: canary
Choose a base branch
from

Conversation

lubieowoce
Copy link
Member

@lubieowoce lubieowoce commented Dec 19, 2024

This PR introduces some extra tracking which makes us treat import(...) like a call to a cached function. Thanks to this, await import(...) will no longer cause dynamicity errors in dynamicIO.
The motivation is the same as allowing fs.readFileSync -- if something is available on the server at prerender time, we don't consider it IO.

Fixes #72589

Implementation notes

The tracking is implemented via an SWC transform (track_dynamic_imports.ts) that turns import(...) into trackDynamicImport(import(...)).

trackDynamicImport(promise) tracks the promise globally, without using cacheSignal. The prospective render subscribes to pending modules using trackPendingModules(cacheSignal), which causes the prospective render to wait for all import()s to finish before proceeding to the final render. The mechanism is analogous to 'use cache', but the "result" of an import() is stored in the module cache instead; when we invoke the import() again in the actual prerender, it will resolve at microtask-speed, like we need it to.

The transform is enabled for all modules that run server-side, both RSC and SSR, because the prerender runs both. This also includes route handlers, because we also use a cacheSignal when prerendering those.
Notably, we also instrument import() in node_modules to account for libraries that do lazy initialization.

I've also had to adjust the prospective client render in PPR mode to wait for cacheSignal.cacheReady() - otherwise, it wouldn't wait for import()s to resolve before doing the final client render, which'd subsequently cause a dynamicity error.
(we didn't need to wait for cacheSignal before, because all cached promises would've already been awaited in the server prerender)

Finally, we no longer need warmFlightResponse, because trackPendingModules already makes the cacheSignal wait for all loading chunks to finish, so we don't need to do it ahead of time.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. tests Turbopack Related to Turbopack with Next.js. type: next labels Dec 19, 2024
@ijjk
Copy link
Member

ijjk commented Dec 19, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
buildDuration 17.9s 16.1s N/A
buildDurationCached 15.2s 12.8s N/A
nodeModulesSize 417 MB 417 MB ⚠️ +23.2 kB
nextStartRea..uration (ms) 470ms 476ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
1187-HASH.js gzip 52.6 kB 52.6 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.44 kB 5.44 kB N/A
bccd1874-HASH.js gzip 52.9 kB 52.9 kB N/A
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 232 B 235 B N/A
main-HASH.js gzip 34.1 kB 34.1 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 0 B 0 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 512 B 510 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.57 kB 4.57 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.35 kB 2.34 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.59 kB 3.59 kB
Client Build Manifests
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
_buildManifest.js gzip 749 B 747 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
index.html gzip 523 B 524 B N/A
link.html gzip 538 B 538 B
withRouter.html gzip 519 B 521 B N/A
Overall change 538 B 538 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 206 kB 206 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
middleware-b..fest.js gzip 671 B 668 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.2 kB 31.2 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
274-experime...dev.js gzip 322 B 322 B
274.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 363 kB 363 kB N/A
app-page-exp..prod.js gzip 129 kB 129 kB
app-page-tur..prod.js gzip 142 kB 142 kB
app-page-tur..prod.js gzip 138 kB 138 kB
app-page.run...dev.js gzip 352 kB 352 kB N/A
app-page.run..prod.js gzip 125 kB 125 kB
app-route-ex...dev.js gzip 37.5 kB 37.5 kB
app-route-ex..prod.js gzip 25.5 kB 25.5 kB
app-route-tu..prod.js gzip 25.5 kB 25.5 kB
app-route-tu..prod.js gzip 25.4 kB 25.4 kB
app-route.ru...dev.js gzip 39.2 kB 39.2 kB
app-route.ru..prod.js gzip 25.4 kB 25.4 kB
pages-api-tu..prod.js gzip 9.69 kB 9.69 kB
pages-api.ru...dev.js gzip 11.6 kB 11.6 kB
pages-api.ru..prod.js gzip 9.68 kB 9.68 kB
pages-turbo...prod.js gzip 21.7 kB 21.7 kB
pages.runtim...dev.js gzip 27.5 kB 27.5 kB
pages.runtim..prod.js gzip 21.7 kB 21.7 kB
server.runti..prod.js gzip 916 kB 916 kB
Overall change 1.73 MB 1.73 MB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js lubieowoce/dynamic-import-cache-tracking Change
0.pack gzip 2.08 MB 2.09 MB ⚠️ +8.45 kB
index.pack gzip 74 kB 74 kB N/A
Overall change 2.08 MB 2.09 MB ⚠️ +8.45 kB
Diff details
Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Commit: 6db6d69

@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch 2 times, most recently from 6041b43 to 38a55f7 Compare December 19, 2024 20:02
@ijjk
Copy link
Member

ijjk commented Dec 19, 2024

Failing test suites

Commit: 1c164e1

pnpm test-dev-turbo test/development/app-hmr/hmr.test.ts (turbopack)

  • app-dir-hmr > filesystem changes > should update server components pages when env files is changed (node-module-var)
Expand output

● app-dir-hmr › filesystem changes › should update server components pages when env files is changed (node-module-var)

expect(received).toEqual(expected) // deep equality

Expected: ArrayContaining [ObjectContaining {"message": StringContaining "[Fast Refresh] done", "source": "log"}]
Received: [{"message": "%cDownload the React DevTools for a better development experience: https://react.dev/link/react-devtools font-weight:bold", "source": "info"}, {"message": "Next.js hydrate callback fired", "source": "log"}, {"message": "connected to ws at ws://localhost:42747/_next/webpack-hmr", "source": "log"}, {"message": "received ws message {\"action\":\"isrManifest\",\"data\":{\"/does-not-exist\":true,\"/folder-renamed\":true,\"/env/node\":true,\"/env/node-module-var\":true}}", "source": "log"}, {"message": "received ws message {\"action\":\"turbopack-connected\",\"data\":{\"sessionId\":3920068871155943}}", "source": "log"}, {"message": "received ws message {\"action\":\"sync\",\"errors\":[],\"warnings\":[],\"hash\":\"\",\"versionInfo\":{\"staleness\":\"fresh\",\"installed\":\"15.4.0-canary.15\"},\"debug\":{},\"devIndicator\":{\"disabledUntil\":0}}", "source": "log"}, {"message": "received ws message {\"action\":\"building\"}", "source": "log"}, {"message": "received ws message {\"action\":\"built\",\"hash\":\"29\",\"errors\":[],\"warnings\":[]}", "source": "log"}, {"message": "received ws message {\"action\":\"serverComponentChanges\",\"hash\":\"30\"}", "source": "log"}, {"message": "received ws message {\"action\":\"building\"}", "source": "log"}, …]

  119 |           await retry(async () => {
  120 |             logs = await browser.log()
> 121 |             expect(logs).toEqual(
      |                          ^
  122 |               expect.arrayContaining([
  123 |                 expect.objectContaining({
  124 |                   message: expect.stringContaining('[Fast Refresh] done'),

  at toEqual (development/app-hmr/hmr.test.ts:121:26)
  at retry (lib/next-test-utils.ts:811:14)
  at development/app-hmr/hmr.test.ts:119:11
  at NextDevInstance.patchFile (lib/next-modes/base.ts:601:9)
  at NextDevInstance.patchFile (lib/next-modes/next-dev.ts:199:16)
  at development/app-hmr/hmr.test.ts:116:9

Read more about building and testing Next.js in contributing.md.

pnpm test-start test/e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts

  • segment cache (basic tests) > navigate to page with lazily-generated (not at build time) static param
Expand output

● segment cache (basic tests) › navigate to page with lazily-generated (not at build time) static param

Expected a response containing the given string:

target-page-with-lazily-generated-param

  104 |     // Reveal the link to trigger a prefetch.
  105 |     const reveal = await browser.elementByCss('input[type="checkbox"]')
> 106 |     const link = await act(
      |                        ^
  107 |       async () => {
  108 |         await reveal.click()
  109 |         return await browser.elementByCss('a')

  at Object.act (e2e/app-dir/segment-cache/basic/segment-cache-basic.test.ts:106:24)

Read more about building and testing Next.js in contributing.md.

@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from 2bc73f4 to 96ab4b5 Compare December 19, 2024 21:45
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from 96ab4b5 to 6db6d69 Compare January 6, 2025 16:54
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch 2 times, most recently from 3b9bb38 to aa83dee Compare January 28, 2025 16:21
Copy link
Member Author

lubieowoce commented Jan 28, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@lubieowoce lubieowoce changed the title fix(dynamicIO): async import() in components transform: instrument async import() Jan 28, 2025
@lubieowoce lubieowoce changed the title transform: instrument async import() transform: cache signal for dynamic import() Jan 28, 2025
@lubieowoce lubieowoce changed the title transform: cache signal for dynamic import() transform: cache tracking for dynamic import() Jan 28, 2025
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from aa83dee to 0fb433f Compare February 3, 2025 12:11
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from 0fb433f to 02aadc1 Compare April 24, 2025 15:52
@lubieowoce lubieowoce changed the title transform: cache tracking for dynamic import() [dynamicIO] cache tracking for import() Apr 24, 2025
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch 9 times, most recently from 1cd26f8 to b5c55e7 Compare April 29, 2025 00:10
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch 3 times, most recently from 31b97c3 to 5b283a6 Compare April 29, 2025 13:40
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch 4 times, most recently from d410aa7 to 8c3b270 Compare April 29, 2025 16:52
@lubieowoce lubieowoce marked this pull request as ready for review April 29, 2025 18:50
@lubieowoce lubieowoce requested review from unstubbable and gnoff April 29, 2025 18:50
@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from 8c3b270 to 4476bbe Compare April 29, 2025 18:55
@@ -7,7 +7,7 @@ export type SchedulerFn<T = void> = (cb: ScheduledFn<T>) => void
*
* @param cb the function to schedule
*/
export const scheduleOnNextTick = <T = void>(cb: ScheduledFn<T>): void => {
export const scheduleOnNextTick = (cb: ScheduledFn<void>) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

just some drive-by cleanups, the T param is unnecessary because it doesn't ever make sense to return a value here

@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from 4476bbe to cd04058 Compare April 29, 2025 19:01
@@ -28,7 +28,7 @@ pids
coverage

# test output
test/**/out*
test/**/out/*
Copy link
Member Author

@lubieowoce lubieowoce Apr 30, 2025

Choose a reason for hiding this comment

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

the previous pattern accidentally matched test/e2e/app-dir/dynamic-io-dynamic-imports/app/outside-of-render

@lubieowoce lubieowoce force-pushed the lubieowoce/dynamic-import-cache-tracking branch from cd04058 to 1c164e1 Compare April 30, 2025 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
created-by: Next.js team PRs by the Next.js team. tests Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug/inconsistency with dynamicIO and dynamic imports
2 participants