Repository navigation
Conversation
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 14, 2026 17:09
6c6289e to
91c031e
Compare
Contributor
size-limit report 📦
|
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 14, 2026 17:57
91c031e to
cc3d3bc
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 14, 2026 18:06
cc3d3bc to
336cf7d
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
2 times, most recently
from
October 14, 2026 20:33
755fc00 to
00d9d69
Compare
JPeer264
added this pull request to stack #25387
October 15, 2026 08:47
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 15, 2026 08:52
00d9d69 to
c448399
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
2 times, most recently
from
October 15, 2026 11:20
f1386d1 to
5c6dd85
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 15, 2026 13:25
5c6dd85 to
ca06931
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
2 times, most recently
from
October 15, 2026 14:57
3ab242c to
621c3a3
Compare
isaacs
force-pushed
the
jp/pnpm-isolated
branch
from
October 15, 2026 19:10
621c3a3 to
307ec62
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 16, 2026 17:30
307ec62 to
bd33f26
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 16, 2026 18:35
bd33f26 to
13548ba
Compare
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
2 times, most recently
from
October 19, 2026 10:06
ee68feb to
94240b6
Compare
JPeer264
marked this pull request as ready for review
October 19, 2026 10:59
s1gr1d
reviewed
Oct 19, 2026
JPeer264
removed this pull request from stack #25387
October 19, 2026 13:42
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 19, 2026 13:43
94240b6 to
4b85a3f
Compare
JPeer264
changed the base branch from
develop
to
jp/node-modules-pnpm-isolated
October 19, 2026 13:44
JPeer264
added this pull request to stack #25528
October 19, 2026 13:44
The test runs a webpack production build. On the Node 20 CI runners it takes 3 to 5 seconds, close to the 5 second default timeout, and some runs time out. Locally it takes about 0.3 seconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Remove `nodeLinker: hoisted` and `shamefullyHoist`, so pnpm installs its default isolated layout. Each package now resolves only the dependencies that it or the root package.json declares. - Packages declare the dependencies that they imported from the hoisted root folder, with the version that the root folder had. The lockfile gets no new versions. - `tslib` is a root devDependency, because the shared tsconfig sets `importHelpers`. - @sentry/replay declares `@sentry/conventions` as a devDependency, so its build still inlines the constant that it imports from there. - react-router, solidstart, sveltekit and tanstackstart-react declare `@types/node` 18 like the root. Without it, the optional `@types/node` peer of Vite resolves to version 20, and the Vite plugin types of @sentry/server-utils do not match. - `packageExtensions` declares the bundlers as optional peers of @apm-js-collab/code-transformer-bundler-plugins, because its type declarations import them. - rollup-utils reads the `sideEffects` field of an external package from the nearest node_modules folder of the built package, not only from the root folder. Without this, the builds keep empty imports of packages that are not in the root folder, for example `import '@sentry/core'`. - The integration test runner adds the bins of node-integration-tests to the `PATH` of setup commands such as `prisma generate`, because the Bun and Deno packages run the shared suites with their own `PATH`. - The install-playwright action runs `npx playwright` outside the repository. In the repository, npm finds the playwright version in `node_modules/.pnpm` and then runs `node_modules/.bin/playwright`, which the root folder no longer has. - The size limit of `@sentry/cloudflare (withSentry)` goes from 514 KiB to 515 KiB. This check bundles without minification, so the bundle keeps the module paths, which are about 4 KB longer under `node_modules/.pnpm`. The code in the bundle is the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The root package.json listed every workspace package as a `workspace:*` devDependency, so that each package could import any of them from the root node_modules folder like with Yarn. Remove these entries and declare the workspace packages in the packages that import them, for example `@sentry-internal/rollup-utils` in the rollup configs. The root keeps three of them: - `@sentry-internal/node-integration-tests`, because its suites import it by its own name, also from tmp folders that have their own package.json. - `@sentry/eslint-plugin-sdk`, because the root oxlint config loads it as a JS plugin. - `@sentry/sveltekit`, because size-limit bundles its build output, which imports `@sentry/sveltekit/browser-tracing-variant`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JPeer264
force-pushed
the
jp/pnpm-isolated
branch
from
October 19, 2026 14:58
4b85a3f to
26a1ddf
Compare
isaacs
approved these changes
Oct 19, 2026
isaacs
left a comment
isaacs
left a comment
Member
There was a problem hiding this comment.
Some minor nits, but this mostly looks good to go.
| }, | ||
| "devDependencies": { | ||
| "premove": "^4.0.0", | ||
| "sentry": "^0.44.0", |
Member
There was a problem hiding this comment.
Thinking this should probably match the lockfile and @sentry/bundler-plugins deps, yeah?
Suggested change
| "sentry": "^0.44.0", | |
| "sentry": "^0.45.0", |
| "meriyah": "^6.1.4", | ||
| "vite": "^8.3.1" | ||
| "vite": "^8.3.1", | ||
| "webpack": "^5.0.0" |
Member
There was a problem hiding this comment.
Suggested change
| "webpack": "^5.0.0" | |
| "webpack": "^5.104.1" |
| sideEffectFree = json.sideEffects === false; | ||
| } catch { | ||
| // Not resolvable from the repo root - assume side effects. | ||
| // Not resolvable from the built package - assume side effects. |
Member
There was a problem hiding this comment.
findPackageJson (down a few lines from here) walks from process.cwd() up to the repo root. If no node_modules/<name>/package.json exists, it returns the root candidate, readFileSync throws, and the package counts as having side effects.
The commit message says this exact fallback caused leftover import '@sentry/core' statements in builds before the fix. With the isolated layout, the fallback now happens whenever a package imports an external that it does not declare directly. The build still succeeds, but the output changes.
Suggestion: log a warning in the catch here, whenever the package is not a Node builtin. Then the next undeclared import shows up in the build log instead of in a later diff of the build output.
| @@ -4,7 +4,7 @@ import { cp, mkdir, readFile, rm, writeFile } from 'fs/promises'; | |||
| import { basename, join } from 'path'; | |||
Member
There was a problem hiding this comment.
Suggested change
| import { basename, join } from 'path'; | |
| import { basename, join, delimiter } from 'path'; |
| // The Bun and Deno packages run these suites with their own `PATH`, which does not have the bins of | ||
| // this package (e.g. `prisma`). | ||
| const binPaths = [join(tmpDirPath, 'node_modules', '.bin'), join(PACKAGE_ROOT, 'node_modules', '.bin')]; | ||
| const env = { ...process.env, PATH: `${binPaths.join(':')}:${process.env.PATH}` }; |
Member
There was a problem hiding this comment.
':' is posix-only. Windows uses ';'. Import path.delimiter and use that instead.
Suggested change
| const env = { ...process.env, PATH: `${binPaths.join(':')}:${process.env.PATH}` }; | |
| const env = { ...process.env, PATH: `${binPaths.join(delimiter)}:${process.env.PATH}` }; |
|
|
||
| # Dependencies that import packages without declaring them. pnpm links these peers to the versions of the importer. | ||
| packageExtensions: | ||
| # Its type declarations import the bundler types. |
Member
There was a problem hiding this comment.
The description says "the real fix belongs upstream".
Suggestion: open an issue on @apm-js-collab/code-transformer-bundler-plugins and link it in the YAML comment here.
This branch has not been deployed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
This moves the monorepo from the hoisted
node_moduleslayout to the isolated pnpm default. Each package now declares the packages that it imports, instead of using what the root folder had. A full build gives the same files as with the hoisted layout.package.jsonno longer lists every workspace package asworkspace:*. Each package declares the workspace packages that it imports, for example@sentry-internal/rollup-utilsin the rollup configs. The root keeps three of them:@sentry-internal/node-integration-tests(its suites import it by name, also from tmp folders with their ownpackage.json),@sentry/eslint-plugin-sdk(oxlint plugin) and@sentry/sveltekit(size-limit bundles its self-import).@sentry/replaydeclares@sentry/conventionsas a dev dependency, so its build still inlines the constant from there.sideEffectsfield of external packages from the nearestnode_modulesof the built package, not only from the root folder.PATHof setup commands (for exampleprisma generate), because the Bun and Deno packages run the shared suites with their ownPATH.npx playwrightoutside the repository. In the repository, npm finds that version innode_modules/.pnpmand then looks fornode_modules/.bin/playwright, which the root folder no longer has.SDK fix
modulesIntegrationdid not find the versions of CJS modules in the isolated pnpm layout (node_modules/.pnpm/<id>/node_modules/<name>), so pnpm apps got the version ranges of theirpackage.jsoninstead. It now reads these folders too. Thesuites/modulesCJS test covers it, because it now runs on this layout.packageExtensions decisions
@apm-js-collab/code-transformer-bundler-pluginsimports the types of Vite, Rollup, webpack and esbuild without declaring them.pnpm-workspace.yamldeclares them as optional peers, so its types use the Vite of the importing package. The real fix belongs upstream.Peer decisions
autoInstallPeersstaysfalse. The pnpm default (true) adds 674 package versions for peers that no package needs, for example@glimmer/componentand the GraphQL tools of Gatsby.Known issues
@sentry/cloudflare (withSentry)entry, but it stays under the 516 KiB limit of develop. Some other entries change by up to 200 bytes. The bundled code is the same, but the bundles contain the longernode_modules/.pnpm/...file paths.