Repository navigation
fix(quill-charts): keep jsx-dev-runtime external in the library build - #127600
Conversation
With a non-production NODE_ENV (vitest sets "test"), vite compiles JSX
to react/jsx-dev-runtime. That module was not external, so the build
bundled its CommonJS code, and the ESM output called require("react").
The check:node-import step then failed, which breaks the MCP integration
tests on every PR.
|
😎 Merged manually by force-merge-posthog[bot] - details. |
|
Risk: No findings One-line build fix: marks Sentinel reviewed |
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
There was a problem hiding this comment.
Approved. Reviewed the PR description, diff, existing discussion, surrounding Vite configuration, React peer dependencies, and ESM/CommonJS import checks. Checked for correctness, security, data safety, performance, missing coverage, overengineering, and scope creep. No issues found. Tests, builds, and linters were not run, as requested.
— Robo Bill v2 (gpt-6.1-sol, high reasoning)
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. ✅ Duplication (Python) — cleanNew Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying. ✅ Duplication (TypeScript) — cleanNew TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying. ✅ Bundle size — no changeUncompressed size of every built Total: 76.14 MiB · no change No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.7 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 4.0 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.9 KiB | src/lib/utils/bootTiming.ts |
| 1.7 KiB | src/index.tsx |
| 1.7 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.2 KiB | src/RootErrorBoundary.tsx |
| 949 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 89.0 KiB | src/lib/api.ts |
| 68.7 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 41.9 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 36.4 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.3 KiB | src/lib/constants.tsx |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 89.0 KiB | src/lib/api.ts |
| 68.7 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 41.9 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 36.4 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 32.7 KiB | src/lib/utils/eventUsageLogic.ts |
| 28.6 KiB | ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js |
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 239.9 KiB | ../packages/quill/packages/quill/dist/index.js |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 144.1 KiB | ../packages/quill/packages/primitives/dist/index.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 94.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 92.6 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 89.0 KiB | src/lib/api.ts |
| 84.9 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/surveys-preview.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 239.9 KiB | ../packages/quill/packages/quill/dist/index.js |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 89.0 KiB | src/lib/api.ts |
| 68.7 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 68.0 KiB | ../products/posthog_ai/frontend/logics/runStreamLogic.ts |
| 41.9 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 36.4 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 239.9 KiB | ../packages/quill/packages/quill/dist/index.js |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 144.1 KiB | ../packages/quill/packages/primitives/dist/index.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 89.0 KiB | src/lib/api.ts |
| 68.7 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 68.0 KiB | ../products/posthog_ai/frontend/logics/runStreamLogic.ts |
| 41.9 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 239.9 KiB | ../packages/quill/packages/quill/dist/index.js |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 144.1 KiB | ../packages/quill/packages/primitives/dist/index.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 94.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 92.6 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 89.0 KiB | src/lib/api.ts |
| 75.3 KiB | src/scenes/dashboard/dashboardLogic.tsx |
Largest files eagerly shipped from src/scenes/web-analytics/WebAnalyticsScene.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 239.9 KiB | ../packages/quill/packages/quill/dist/index.js |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 144.1 KiB | ../packages/quill/packages/primitives/dist/index.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 94.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 92.6 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 89.0 KiB | src/lib/api.ts |
| 84.9 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/surveys-preview.js |
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
| Size | File |
|---|---|
| 325.3 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 320.2 KiB | ../node_modules/.pnpm/posthog-js@1.444.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 239.9 KiB | ../packages/quill/packages/quill/dist/index.js |
| 224.1 KiB | ../node_modules/.pnpm/@posthog+icons@0.39.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.2 KiB | src/products.tsx |
| 94.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 92.6 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 89.0 KiB | src/lib/api.ts |
| 68.7 KiB | src/lib/lemon-ui/icons/icons.tsx |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Frontend layers — no new upward imports, 0 removed
Static imports from a lower frontend layer into a higher one (layers, lowest first: kernel, shell, query, features). Compared with the base branch, so an import that a PR also adds to layer-baseline.txt still shows here. Sizes are the source bytes on the eager path of every logged-in page.
| Layer | Eager source bytes | Δ vs base |
|---|---|---|
| kernel | 2.96 MiB | no change |
| shell | 2.38 MiB | no change |
| query | 56.4 KiB | no change |
| features | 534.0 KiB | no change |
Download the interactive layer graph for this PR.
Posted automatically by check-layers
✅ Toolbar bundle — eager 2.34 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.34 MiB · 19 files | no change | ████░░░░░░ 40.9% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 42 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 944.9 KiB | dist/toolbar/toolbar-app-5IRQLJNF.css |
| 696.2 KiB | dist/toolbar/chunk-chunk-EKT3X5Y5.js |
| 261.2 KiB | dist/toolbar/chunk-chunk-JU6PYZZI.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 130.3 KiB | dist/toolbar/chunk-chunk-KOKWFYFT.js |
| 75.7 KiB | dist/toolbar/toolbar-app-HU7UNWPX.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 33.8 KiB | dist/toolbar/chunk-chunk-F5VUPTPI.js |
| 21.7 KiB | dist/toolbar/chunk-chunk-U4EGNVMX.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — no change
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 969.76 MiB · no change
ℹ️ MCP UI apps size — 35 app(s), 29126.5 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 844.4 KB | 211.6 KB |
| explain-chart | 1119.1 KB | 211.6 KB |
| action | 767.8 KB | 211.6 KB |
| action-list | 835.4 KB | 211.6 KB |
| cohort | 766.8 KB | 211.6 KB |
| cohort-list | 834.4 KB | 211.6 KB |
| email-template | 766.6 KB | 211.6 KB |
| error-details | 771.7 KB | 211.6 KB |
| error-issue | 769.0 KB | 211.6 KB |
| error-issue-list | 836.8 KB | 211.6 KB |
| experiment | 832.5 KB | 211.6 KB |
| experiment-list | 836.1 KB | 211.6 KB |
| experiment-results | 834.3 KB | 211.6 KB |
| feature-flag | 834.8 KB | 211.6 KB |
| feature-flag-list | 838.5 KB | 211.6 KB |
| feature-flag-testing | 770.0 KB | 211.6 KB |
| inline-scan | 767.2 KB | 211.6 KB |
| insight-actors | 833.8 KB | 211.6 KB |
| llm-costs | 831.1 KB | 211.6 KB |
| session-recording | 769.0 KB | 211.6 KB |
| survey | 768.4 KB | 211.6 KB |
| survey-global-stats | 831.2 KB | 211.6 KB |
| survey-list | 836.1 KB | 211.6 KB |
| survey-stats | 831.2 KB | 211.6 KB |
| trace-span | 767.1 KB | 211.6 KB |
| trace-span-list | 835.3 KB | 211.6 KB |
| vision-observation-list | 834.5 KB | 211.6 KB |
| workflow | 767.1 KB | 211.6 KB |
| workflow-list | 834.7 KB | 211.6 KB |
| workflow-setup-status | 766.8 KB | 211.6 KB |
| query-results | 1090.8 KB | 211.6 KB |
| render-ui | 1195.0 KB | 211.6 KB |
| visual-review-snapshots | 771.6 KB | 211.6 KB |
| workflow-data-suggestions | 769.4 KB | 211.6 KB |
| workflow-sending-rules | 768.1 KB | 211.6 KB |
There was a problem hiding this comment.
Approved.
The build-tooling change has independent assurance from @bill-ph's current-head approval. No unresolved substantive concerns or other refusal grounds were found.
- PostHog Review (Standard) reviewed the current head: no findings.
- bill-ph reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 1L, 1F substantive — within ceiling |
| tier | ✓ | T1-agent / T1a-trivial (1L, 1F, single-area, fix) |
| stamphog 2.7.0 | .stamphog/policy.yml @ d4ad7ac · reviewed head d4ad7ac |
HostHog preview —
|
benben
left a comment
There was a problem hiding this comment.
Automated review generated on behalf of @benben.
Approve. One-line fix externalizing react/jsx-dev-runtime in quill-charts' lib build; verified the externals list, the react peer range covers the subpath, and the MCP CI chain (vitest NODE_ENV=test → build:ui-apps → build:quill → charts' check:node-import). Production output is unaffected since the module never appears in prod builds, and the check passes for the ESM and CJS halves under any NODE_ENV. The same external gap remains in the four sibling quill packages, whose dev/test-built dists feed the MCP ui-app bundles in that environment.
- major
packages/quill/packages/components/vite.config.ts:24Sibling quill packages still bundle jsx-dev-runtime. Under NODE_ENV=test the vitest-spawned MCP build makes blocks/primitives/quill-components dists bundle react/jsx-dev-runtime, reproducing the bare-require-in-ESM failure charts just fixed; the quill aggregate inlines those dists (react/jsx-runtime external only, vite.config.ts:52) into dist/index.js, which the MCP ui-app IIFE bundles then inline — and only charts has check:node-import to catch it.
Model: us.zai.glm-5.3.
|
/trunk merge |
|
/trunk merge --priority=0 |
|
Warning A break-glass force-merge has been requested for this PR. Initiated by: Paul D'Ambra (Slack) Branch protection requirements will be bypassed. If this was unexpected, contact #team-security. |
|
✅ Force-merge completed. |
Problem
@posthog/quill-chartswith vitest'sNODE_ENV=test.NODE_ENV, vite compiles JSX toreact/jsx-dev-runtime. That module was not in theexternallist, so the build bundled its CommonJS code.require("react")inside the ESM output. The newcheck:node-importstep catches this and fails the build.Changes
react/jsx-dev-runtimeis now external, next toreact/jsx-runtime.reactinstead of bundling it.How did you test this code?
NODE_ENV=test pnpm buildinpackages/quill/packages/charts. The build sizes matched the failing CI run.pnpm buildpassescheck:node-importwithNODE_ENVset totest,development, and unset.NODE_ENV=test pnpm run build:ui-appsinservices/mcppasses.Test rationale:
check:node-importfrom #125541 already guards this. The fix makes that check pass in the environment CI uses, so no new test is needed.Release status
Docs update
None.