-
Notifications
You must be signed in to change notification settings - Fork 0
Sync upstream v2.10.3 (merge conflicts) #30
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
base: main
Are you sure you want to change the base?
Changes from all commits
c66c6e7
1a39a49
ee4b368
b11a7ce
10d8428
e034e9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| # 0001 — webpack for production builds, Turbopack for dev | ||
|
|
||
| | | | | ||
| | --- | --- | | ||
| | Status | accepted | | ||
| | Date | 2026-08-04 | | ||
| | Deciders | @tom2drum | | ||
| | Supersedes | — | | ||
|
|
||
| ## Decision | ||
|
|
||
| **Production builds use webpack (`next build --webpack`). Dev keeps Turbopack (the Next 16 default).** | ||
|
|
||
| Applies to every entry point that emits a production bundle: | ||
|
|
||
| | Entry point | Bundler | | ||
| | --- | --- | | ||
| | `pnpm build` — what the `Dockerfile` runs for the shipped image | webpack | | ||
| | `pnpm build:next` | webpack | | ||
| | `pnpm prod:preset <alias>` — local production build, incl. perf measurements | webpack | | ||
| | `pnpm build:analyze`, `pnpm profile:preset` | webpack (already were) | | ||
| | `pnpm dev`, `pnpm dev:preset`, `pnpm dev:local` | Turbopack | | ||
|
|
||
| Dev stays on Turbopack because it is roughly 3× faster to compile and the crash class below only | ||
| manifests in a minified production build. Production-build regressions are caught in QA rather than | ||
| by making every local dev start slower. | ||
|
|
||
| ## Why | ||
|
|
||
| ### Turbopack miscompiles the Dynamic-labs SDK | ||
|
|
||
| Turbopack's scope hoisting emits code that reads the SDK's `UserFieldEditorContext` through the | ||
| wrong binding. `useContext` therefore receives a non-context value, returns `undefined`, and the SDK | ||
| throws from its own `useUpdateUserWithModal`: | ||
|
|
||
| ``` | ||
| useUserUpdateRequest can only be used inside the context of DynamicContextProvider | ||
| ``` | ||
|
|
||
| The throwing component is the SDK's internal `SyncAuthFlow`, which the SDK itself renders *inside* | ||
| `UserFieldEditorContextProvider` — so in a correct build the context cannot be missing. It is a | ||
| bundler defect, not a provider-tree bug in our code. | ||
|
|
||
| Impact: **a hard crash on the initial load of every page**, for any instance configured with | ||
| `NEXT_PUBLIC_ACCOUNT_AUTH_PROVIDER=dynamic`. It is invisible in dev (unminified, no hoisting) and | ||
| was found only by running the `v2.10.0` image locally. The v2.10.0 release would have broken every | ||
| dynamic-auth instance on rollout; deployed instances were still on v2.9.4 and unaffected. | ||
|
|
||
| Bisected to [#3574](https://github.com/blockscout/frontend/pull/3574) (wallet-stack deferral, | ||
| subtask 4 of [#3566](https://github.com/blockscout/frontend/issues/3566)) — parent commit good, that | ||
| commit bad. The trigger could **not** be reduced to a single import: reverting the lazy `import()` | ||
| wrappers, the `_app.tsx` provider restructure, and the `@wagmi/core` dependency each left it broken. | ||
| That fits the mechanism — scope hoisting groups modules across the whole graph, so the trigger is an | ||
| emergent property of how #3574 reshaped it, and any future graph change could re-trigger it | ||
| somewhere else. Next 16.3.0 does not fix it. | ||
|
|
||
| ### webpack is also the faster bundle | ||
|
|
||
| Two options fixed the crash: `--webpack`, or `experimental.turbopackScopeHoisting: false`. The flag | ||
| turned out to be the expensive one. Production builds of `main`, medians of 3 | ||
| automated traces: | ||
|
|
||
| | Metric | Turbopack | Turbopack, hoisting off | **webpack** | | ||
| | --- | --- | --- | --- | | ||
| | M1 FCP | 432 ms | 790 ms | **501 ms** | | ||
| | M2 first API request | 60 ms | 142 ms | **57 ms** | | ||
| | M5 blocking time | 133 ms | 408 ms | **155 ms** | | ||
| | M6 JS before FCP | 1038 KB | 1064 KB | **697 KB** | | ||
| | Emitted chunk bytes | 49.2 MB | 53.4 MB | **21.4 MB** | | ||
| | Build time | 48 s | 41 s | 2.4 min | | ||
|
|
||
| Disabling scope hoisting nearly doubles FCP and triples blocking time while barely moving M6 (+2.5%) | ||
| — the cost lands in execution, not transfer, so M6 alone would not have caught it. webpack instead | ||
| *improves* pre-FCP JS by 341 KB (−33%) over the Turbopack build, more than any single lever in #3566 | ||
| delivered on its own. | ||
|
|
||
| The measurement harness lives in | ||
| `.agents/tasks/3566-main-page-loading-perf/tools/` (see its README). Absolute values come from | ||
| headless Chromium on a local server and are not comparable to the numbers in that task's spec table; | ||
| the within-comparison deltas are what the decision rests on. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - **CI and image builds get slower** — webpack's compile step measured 84 s to 2.4 min across | ||
| machines and cache states, against 41–48 s for Turbopack, so budget roughly 2–3×. Accepted: | ||
| correctness plus a materially smaller bundle outweigh build latency. | ||
| - **Dev and production now use different bundlers.** A bug in either pipeline can only be caught on | ||
| that pipeline; production-only breakage will not appear in dev. QA runs against a real image. | ||
| - `next.config.js` must keep **both** the `webpack()` and `turbopack` sections in sync — it already | ||
| does, and this decision makes that non-optional. | ||
| - webpack surfaces one unresolvable import Turbopack silently tolerates: | ||
| `@react-native-async-storage/async-storage` inside `@metamask/sdk`, reached via | ||
| `@wagmi/connectors` → `@reown/appkit-adapter-wagmi` → `wagmi-config.ts`. It is an optional peer | ||
| dependency of a React Native code path a browser bundle never takes, so `next.config.js` maps it | ||
| to `false` in `resolve.fallback` (an empty module) and the build is warning-free. If a future | ||
| dependency bump introduces a similar optional import, extend that map rather than silencing | ||
| warnings wholesale. | ||
| - `next build --webpack` is a compatibility path in Next 16 and may eventually be removed. If that | ||
| happens before Turbopack is fixed, the fallback is `experimental.turbopackScopeHoisting: false` | ||
| and its performance cost. | ||
|
|
||
| ## Follow-ups | ||
|
|
||
| - Report the miscompilation upstream to `vercel/next.js` with a minimal reproduction; the bisect | ||
| boundary and the flag that toggles it are the material. | ||
| - Re-test Turbopack on each Next upgrade. If a release fixes it, revisit — Turbopack's build speed | ||
| is worth reclaiming, but only with the M1/M5/M6 numbers above re-measured, not on the release | ||
| notes alone. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| // Record page-load performance traces without the DevTools UI. | ||
| // | ||
| // Produces the same JSON the Performance panel's "Record and reload" export produces, so the output | ||
| // feeds straight into trace-metrics.py. Use it when you want several runs per variant (M3/M4 need a | ||
| // median) or a repeatable A/B — the manual protocol in README.md is still fine for a one-off. | ||
| // | ||
| // Usage, against an already-running production server (`pnpm prod:preset <alias>`): | ||
| // | ||
| // node .agents/tasks/3566-main-page-loading-perf/tools/trace.mjs http://localhost:3000/ ./traces/after 3 | ||
| // python3 .agents/tasks/3566-main-page-loading-perf/tools/trace-metrics.py ./traces/before-2.json ./traces/after-2.json | ||
| // | ||
| // Writes <out-prefix>-<n>.json for n in 1..runs. | ||
|
|
||
| /* eslint-disable no-console -- a CLI tool: stdout is its interface, for the usage hint and for | ||
| reporting each trace it wrote. */ | ||
|
|
||
| import { chromium } from '@playwright/test'; | ||
| import { mkdirSync, writeFileSync } from 'node:fs'; | ||
| import { dirname } from 'node:path'; | ||
|
|
||
| // Long enough for the transactions/blocks lists to fill with real data, which M3/M4 measure. | ||
| const SETTLE_MS = 10_000; | ||
|
|
||
| // The capture set the Performance panel uses: devtools.timeline for resources/tasks/render commits, | ||
| // loading for navigationStart, blink.user_timing for paint marks, __metadata for thread names | ||
| // (trace-metrics.py needs those to tell CrRendererMain apart from other threads). | ||
| const CATEGORIES = [ | ||
| '-*', | ||
| 'devtools.timeline', | ||
| 'disabled-by-default-devtools.timeline', | ||
| 'disabled-by-default-devtools.timeline.frame', | ||
| 'blink.user_timing', | ||
| 'loading', | ||
| 'latencyInfo', | ||
| 'v8.execute', | ||
| '__metadata', | ||
| ]; | ||
|
|
||
| const [ url, outPrefix, runsArg ] = process.argv.slice(2); | ||
| if (!url || !outPrefix) { | ||
| console.error('Usage: node trace.mjs <url> <out-prefix> [runs=1]'); | ||
| process.exit(2); | ||
| } | ||
| const runs = Number(runsArg ?? 1); | ||
|
|
||
| mkdirSync(dirname(outPrefix), { recursive: true }); | ||
|
|
||
| // A fresh context per run is the scripted equivalent of the protocol's "clean browser profile": | ||
| // no extensions, no warm HTTP cache, no carried-over service worker. | ||
| const browser = await chromium.launch(); | ||
|
|
||
| for (let run = 1; run <= runs; run++) { | ||
| const context = await browser.newContext(); | ||
| const page = await context.newPage(); | ||
| const client = await context.newCDPSession(page); | ||
|
|
||
| const events = []; | ||
| client.on('Tracing.dataCollected', ({ value }) => events.push(...value)); | ||
| const complete = new Promise((resolve) => client.once('Tracing.tracingComplete', resolve)); | ||
|
|
||
| // Tracing has to start before the navigation — that is what "Record and reload" does, and | ||
| // navigationStart is the zero point every metric is relative to. | ||
| await client.send('Tracing.start', { | ||
| transferMode: 'ReportEvents', | ||
| traceConfig: { includedCategories: CATEGORIES, recordMode: 'recordAsMuchAsPossible' }, | ||
| }); | ||
|
|
||
| await page.goto(url, { waitUntil: 'load', timeout: 60_000 }); | ||
| await page.waitForTimeout(SETTLE_MS); | ||
|
|
||
| await client.send('Tracing.end'); | ||
| await complete; | ||
| await context.close(); | ||
|
|
||
| const out = `${ outPrefix }-${ run }.json`; | ||
| writeFileSync(out, JSON.stringify({ traceEvents: events })); | ||
| console.log(`${ out }: ${ events.length } events`); | ||
| } | ||
|
|
||
| await browser.close(); | ||
|
Comment on lines
+50
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,110p' .agents/tasks/3566-main-page-loading-perf/tools/trace.mjs
printf '\n--- package/version references ---\n'
rg -n 'playwright|chromium|Tracing\.end|context\.close|browser\.close' .agents/tasks/3566-main-page-loading-perf package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null | head -120Repository: DOS/DOScan-Frontend Length of output: 7990 Close tracing resources on failure. If 🤖 Prompt for AI Agents |
||
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject invalid run counts.
Number(runsArg ?? 1)accepts zero, negative, fractional,NaN, andInfinity. These values can create no traces, create an unexpected number of traces, or make the loop unbounded. Validate a positive integer beforemkdirSync.Proposed validation
📝 Committable suggestion
🤖 Prompt for AI Agents