[pull] canary from vercel:canary - #1318
Merged
Merged
Conversation
## Summary After a fallback-shell cache prerender completes, snapshot whether its timeout fired and then abort the existing timeout controller when it participates in an `AbortSignal.any()` composite. This triggers the composite so React removes its abort listener; no additional controller or signal is needed. Cache prerenders without a dynamic-access source keep their existing direct timeout signal. Node retains non-empty composite abort signals while they have abort listeners. React attaches such a listener during `prerender()` and removes it when the signal aborts, so aborting the already-owned timeout source releases the successful render. Snapshotting `didTimeout` first keeps cleanup aborts distinct from real timeouts. This preserves the early aborted-prerender guard from #96426, which prevents a cache fill that starts after its outer prerender aborts from caching an empty React stream. Fixes #97363 Related #97464 Alternative to #97391 ## Verification - `pnpm --filter=next build` - `pnpm test-start-turbo test/e2e/app-dir/use-cache-after-uncached-io/use-cache-after-uncached-io.test.ts` - `pnpm test-start-turbo test/e2e/app-dir/use-cache-hanging/use-cache-hanging.test.ts` - Actual vendored React `prerender()` GC probe on Node 20.19.5 and 22.20.0: valid preludes, no cleanup errors, and 0/100 composite signals retained while their source controllers remained reachable. <!-- NEXT_JS_LLM -->
LCP can be affected if the LCP element is sitting within a Suspense boundary, even if its nothing but static content, if the static content is "too large". Apps should prefer, as long as possible, to not have a Suspense boundary around the LCP element. - https://react.dev/reference/react/Suspense#what-activates-a-suspense-boundary > Waiting for a large boundary’s HTML to arrive during streaming server rendering. Sending HTML takes time, so a boundary with enough content activates even when nothing in it suspends. React reveals the content as the HTML arrives.
these were failing with 50% probability since today, for some reason: https://app.datadoghq.com/ci/test/runs?query=test_level%3Asuite%20%40test.service%3Anextjs%20%40test.type%3Aturbopack%20%40test.suite%3Aconcurrent-install&agg_m=count&agg_m_source=base&agg_t=count&fromUser=false&start=1786528015972&end=1787132815972&paused=false
Closes #92082 This is now a proper compile-time constant: ```js import { IS_DEV } from './other' if (IS_DEV) { // statically evaluates to `true` console.log('x') } else { require("library") // not bundled } console.log(IS_DEV); // is replaced with console.log(true); ``` ```typescript // other.ts export const SOME_VALUE = 'x' const node_env = process.env.NODE_ENV const development_ent = 'development' export const IS_DEV = node_env === development_ent ``` You can use code to compute constants just fine, and use any existing constants such as `process.env.NODE_ENV`. Currently, you can't use imports to other constants modules, but we can add that later on. We can't perform this constants check for every single import, so you need to either - have `UPPER_CASE` import names as in the example above - or use `import { lower } from './other' with { turbopackConstants: 'true' }` Then that referenced module will be analyzed for constants, and if the referenced export is a constant, it will participate in constant inlining just as `process.env.FOO`. This also works fine with barrel imports, you can still do `import { IS_DEV } from './barrel.js'` and it will find the `constants.js` file which in itself will indeed only have constants exports. Because it's not always opt-in at the import site, we can't automatically make non-constant exports an error. For that you have to add `'use turbopack: constants'` at the top of the module, which will make it an error if any constant import references that module, and the module has any non-constant exports: ``` error - [analysis] /turbopack/crates/turbopack-tests/tests/snapshot/comptime/cross-module/input/other.constants.js:8:7 Export NO_CONSTANT is not a constant 4 | const development_ent = 'development' 5 | 6 | export const IS_DEV = node_env === development_ent 7 | + v--------------------------------v 8 + export const NO_CONSTANT = globalThis.foo + ^--------------------------------^ 9 | It was analyzed to be FreeVar(globalThis)["foo"] Import trace: test: ./turbopack/crates/turbopack-tests/tests/snapshot/comptime/cross-module/input/other.constants.js ./turbopack/crates/turbopack-tests/tests/snapshot/comptime/cross-module/input/index.js ``` Some prior art: https://rspack.rs/blog/announcing-1-5#const-inline-optimization, https://rspack.rs/config/optimization#optimizationinlineexports No compile-time impact on a big app: ``` canary dfbc3dc: 438.11s user, 68.35s system, 801% cpu, 1:03.22 total 439.44s user, 71.47s system, 756% cpu, 1:07.56 total 440.32s user, 68.57s system, 750% cpu, 1:07.81 total constants 2d7eb82 433.22s user, 65.44s system, 800% cpu, 1:02.32 total 440.52s user, 67.68s system, 770% cpu, 1:05.95 total 433.13s user, 69.47s system, 776% cpu, 1:04.74 total ``` a8620556af64ef550373cca1e1fc701a1a8583cc
example failure: https://github.com/vercel/next.js/actions/runs/32246702678/job/96048986756?pr=90300 datadog: https://app.datadoghq.com/ci/test/runs?query=test_level%3Asuite%20%40git.repository.id%3A%22github.com%2Fvercel%2Fnext.js%22%20%40test.suite%3A%22app-dir%20-%20error-on-next-codemod-comment%22&agg_m=%40duration&agg_m_source=base&agg_t=avg&fromUser=false&index=citest&viz=stream&start=1786535051579&end=1787139851579&paused=false 1. Use the callback form of `patchFile` for automatic and more robust cleanup 2. Longer `retry` duration in case HMR is slow on CI
No reason not to allow this in release builds
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )