deps: migrate pnpm internals to v11 (1100.x)#10293
Open
zkochan wants to merge 44 commits into
Open
Conversation
- Rename 17 @pnpm/* packages in workspace.jsonc to new domain-based names (e.g. @pnpm/core → @pnpm/installing.deps-installer, @pnpm/config → @pnpm/config.reader), bump in-monorepo packages to 1100.0.0. - Update TS imports across scopes/components/e2e. - mutateModules: replace removed neverBuiltDependencies/ onlyBuiltDependencies/ignoredBuiltDependencies with the v11 allowBuilds map + dangerouslyAllowAllBuilds flag. - Config.rawConfig → Config.authConfig; read network settings from typed Config fields instead of rawConfig ini lookups. - Convert flat auth rawConfig to the new configByUri: Record<string, RegistryConfig> shape for CreateStoreControllerOptions; switch generateResolverAndFetcher to createResolver (avoids the now-required storeIndex on createClient). - Rename sortPackages → sortProjects, createPkgGraph → createProjectsGraph, createOrConnectStoreController → createStoreController; supply globalVirtualStoreDir to getPeerDependencyIssues.
- Replace @pnpm/installing.modules-yaml imports in e2e tests with a local readModulesManifest that parses .modules.yaml directly. The pnpm v11 package is ESM and fails with 'Cannot require() ES Module ... because it is not yet fully loaded' when Mocha loads multiple test files in parallel. - Drop @pnpm/network.fetch from npm-ci-registry.ts and use the built-in global fetch with a local retry loop.
The pnpm v11 packages are ESM, and the `teambit.harmony/envs/core-aspect-env` Mocha
runner in the build capsule fails with "Cannot use import statement outside a module"
when the spec files transitively `require('@pnpm/deps.path' | '@pnpm/lockfile.fs' | '@pnpm/error')`.
Running `bit test scopes/dependencies/pnpm` locally still passes (10/10), so the
coverage isn't lost for local development — just disabled in CI until the capsule
test env supports require() of ESM.
Pnpm v11 (#11189) restricts .npmrc to auth + registry settings only; hoist-pattern now lives in pnpm-workspace.yaml, so the legacy test premise no longer applies.
pnpm v11 removed the neverBuiltDependencies install option entirely. Bit's public API still exposes it, so translate it into the new allowBuilds map with values set to false. Also ensures dangerouslyAllowAllScripts respects the blocklist.
pnpm v11's createAllowBuildFunction short-circuits when dangerouslyAllowAllBuilds is true and ignores allowBuilds entries entirely. To keep the 'allow all except X' pattern working, skip the flag when a deny list is present and emit allowBuilds with just the deny entries.
The package is ESM in pnpm v11; loading it through a top-level CJS require() in the build capsule crashes with 'Unexpected token export'. Dynamic import bypasses Node's require-ESM machinery and uses the proper ESM loader.
Using new Function('return import(p)') prevents bit's compiler from
rewriting dynamic import() back to require(), so @pnpm/releasing.commands
truly loads through Node's ESM loader rather than require-ESM.
…lude
Exclude @babel/plugin-transform-dynamic-import in both the root babel
config and the aspect env babel config so Babel preserves native
import() in compiled CJS output. This lets packer.ts call
import('@pnpm/releasing.commands') directly instead of going through
the new Function('return import(p)') escape hatch.
Now that babel preserves native import() in CJS output, the inlined
readModulesManifest in three e2e tests and the hand-rolled
fetchWithRetry in npm-ci-registry can be dropped in favor of
await import('@pnpm/installing.modules-yaml') / '@pnpm/network.fetch'
inside the call sites. Type-only imports of Modules stay static
since they're stripped at compile time.
The legacy link is only needed for external repos still importing `@teambit/legacy`. Since v11 no longer guarantees the package is installed locally (pnpm-lock no longer carries it), throwing on require.resolve aborts every install/link in fresh CI workspaces. Swallow the resolution failure so linking continues.
Three pre-existing dead statements (a `throw` after `return`/`throw` in each file) only started failing once the post-install oxlint bump from 1.12 to 1.62 enabled stricter `no-unreachable` detection.
Dynamic import() in TypeScript sources is rewritten to a require()-wrapped Promise by @babel/plugin-transform-modules-commonjs, regardless of the preset-env exclude. Move the import() call into a sibling .cjs file that the babel compiler skips (isFileSupported is false for .cjs), so the native dynamic import survives compilation and pnpm's ESM-only package loads correctly inside the build capsule.
….yaml pnpm v11 (#11189) restricts .npmrc to auth + registry settings; behaviour flags such as hoist-pattern moved to pnpm-workspace.yaml. Add a pnpmWorkspaceConfig option to the scope helper and switch the test to write hoist-pattern there, restoring the assertion that workspace-level hoist-pattern propagates into the capsule install.
Pare back the comments added during the v11 work to keep only the non-obvious "why" notes — drop what-comments, JSDoc/inline filler in the auth-config helper, the duplicate ESM justification on packer.ts, and the empty-catch placeholder in dependency-linker.ts.
The premise — workspace .npmrc options propagating into capsule installs — no longer applies under pnpm v11, and bridging this through pnpm-workspace.yaml isn't worth supporting. Drop the test and the pnpmWorkspaceConfig helper hook added for the rewrite attempt.
…ompat" This reverts the spec deletions from 9fa93e8. Node 22's native require(esm) lets the capsule's mocha runner load the transitive ESM-only @pnpm/* deps without any wrapping; bit test runs all 12 specs successfully.
…ipping on ESM) Local bit test passes on Node 22 thanks to require(esm), but the core-aspect-env mocha capsule on CI still throws "Cannot use import statement outside a module" on the same Node 22.22 image. The spec's top-level import of `./lockfile-deps-graph-converter` triggers the chain unconditionally, so describe.skip does not help — drop the spec again until the prod module's ESM-only @pnpm/deps.path and @pnpm/lockfile.fs imports get pushed through a .cjs shim. Keeps pnpm-error-to-bit-error.spec.ts which only references @pnpm/error via a type-only import.
The capsule mocha-tester pins @babel/register@7.18.9 (2022), whose require hook trips Node's require(esm) path on transitive @pnpm/deps.path / @pnpm/lockfile.fs loads. Forcing ^7.28 via workspace.jsonc overrides applies the rewritten compile/worker implementation, which preserves the require(esm) flow for ignored paths. Restore the spec so CI can verify; bit test passes locally with the override (12/12).
@pnpm/config.reader@1101.2.0 exposes the helpers pnpm itself uses to turn a flat npmrc-style auth dict into the configByUri map. Drop the local port (authConfigToConfigByUri + decodeBasicAuth) and call the upstream functions directly so the parsing stays in lockstep with pnpm — including basicAuth decoding, SSL key/cert/file handling, and default-registry creds keying.
The @babel/register@^7.28 override in workspace.jsonc only applies to the bit workspace install; the env capsule that runs MochaTest installs its own deps from @teambit/defender.mocha-tester (which pins @babel/register@7.18.9), so the override never reached the failing runner. Newer @babel/register also still trips on the transitive require(esm) chain in CI. Removing both the override and the lockfile spec, leaving the small pnpm-error-to-bit-error.spec.ts which is unaffected. Restoring the spec needs a fix in @teambit/defender.mocha-tester (or a capsule-level override mechanism).
…tern Move the @pnpm/deps.path and @pnpm/lockfile.fs imports behind an exported async init() that loads them through a .cjs shim so the build capsule's mocha runner doesn't trip on the transitive ESM require chain. Helpers keep using module-level slots synchronously, so convertLockfileToGraph / convertGraphToLockfile and all internal helpers stay sync in shape. Callers (pnpm.package-manager.ts:calcDependenciesGraph and dependenciesGraphToLockfile) await init() once. The spec adds a single top-level before(init) plus a small before() in the simple-case describe that needed to defer its describe-body-level convertLockfileToGraph call until after init runs. bit test passes 12/12 locally.
- lockfile-deps-graph-converter: cache the in-flight load promise so concurrent init() calls share the same single ESM load instead of racing each other and producing transient undefined dp/getLockfileImporterId. - pkg-manager-config.e2e: rename the suite to reflect that only the Yarn rc-file case remains (the pnpm hoist-pattern variant was dropped with v11 since pnpm-workspace.yaml is the supported config path now).
Comment on lines
1
to
4
| import semver from 'semver'; | ||
| import type { LockfilePackageInfo } from '@pnpm/lockfile.types'; | ||
| import * as dp from '@pnpm/dependency-path'; | ||
| import * as dp from '@pnpm/deps.path'; | ||
|
|
Comment on lines
10
to
13
| (supportNpmCiRegistryTesting ? describe : describe.skip)( | ||
| 'package manager rc file is read from the workspace directory when installation is in a capsule', | ||
| 'workspace .yarnrc.yml is read by Yarn when installation is in a capsule', | ||
| function () { | ||
| this.timeout(0); |
| import { buildDependentsTree } from '@pnpm/reviewing.dependencies-hierarchy'; | ||
| import { renderDependentsTree } from '@pnpm/list'; | ||
| import type { Modules } from '@pnpm/installing.modules-yaml'; | ||
| import { readModulesManifest } from '@pnpm/installing.modules-yaml'; |
# Conflicts: # pnpm-lock.yaml
- Pin @pnpm/config.env-replace to 3.0.2 via overrides — bit.cloud's registry mirror does not have the upstream 4.x versions that newer @pnpm/config.reader requires.
Newer config.reader (1101.3.2+) imports envReplaceLossy from @pnpm/config.env-replace@^4.x, but bit.cloud's registry mirror only serves env-replace up to 3.0.2 (where envReplaceLossy doesn't exist). The previous override of @pnpm/config.env-replace let install pass but broke runtime with "does not provide an export named 'envReplaceLossy'". Forcing config.reader itself to 1101.3.1 keeps it on the env-replace 3.x API that bit.cloud has.
…oken ESM chain
Loading @pnpm/releasing.commands' index.js eagerly evaluates its deploy
re-export, whose transitive chain hits @pnpm/installing.commands' import
module — which uses `import { parse } from '@yarnpkg/lockfile'`.
@yarnpkg/lockfile is a webpack-bundled CJS module that Node's ESM-CJS
bridge can't statically detect named exports from, so the whole tree
fails to evaluate.
The pack module itself has no imports from that subtree. Resolve the
package's main entry, then load pack.js by absolute file URL, skipping
index.js entirely.
Comment on lines
1
to
4
| import semver from 'semver'; | ||
| import type { LockfilePackageInfo } from '@pnpm/lockfile.types'; | ||
| import * as dp from '@pnpm/dependency-path'; | ||
| import * as dp from '@pnpm/deps.path'; | ||
|
|
Comment on lines
10
to
12
| (supportNpmCiRegistryTesting ? describe : describe.skip)( | ||
| 'package manager rc file is read from the workspace directory when installation is in a capsule', | ||
| 'workspace .yarnrc.yml is read by Yarn when installation is in a capsule', | ||
| function () { |
Comment on lines
1
to
4
| import semver from 'semver'; | ||
| import type { LockfilePackageInfo } from '@pnpm/lockfile.types'; | ||
| import * as dp from '@pnpm/dependency-path'; | ||
| import * as dp from '@pnpm/deps.path'; | ||
|
|
Comment on lines
10
to
12
| (supportNpmCiRegistryTesting ? describe : describe.skip)( | ||
| 'package manager rc file is read from the workspace directory when installation is in a capsule', | ||
| 'workspace .yarnrc.yml is read by Yarn when installation is in a capsule', | ||
| function () { |
promise-share 2.0.0 is "type": "module" with no "exports"/"main" field, which triggers a DEP0151 deprecation warning when @pnpm/installing.* import it as ESM. 2.0.1 adds the "exports" field. Pinned via an override.
Comment on lines
+25
to
+31
| // @pnpm/deps.path and @pnpm/lockfile.fs are ESM-only; load them through a .cjs | ||
| // shim so the require() chain in the build capsule's mocha runner doesn't trip | ||
| // on the transitive ESM import. Call `init()` once before invoking the public | ||
| // converters; helpers reach for these module-level slots synchronously. | ||
| let dp!: typeof Dp; | ||
| let getLockfileImporterId!: typeof GetLockfileImporterId; | ||
| let loading: Promise<void> | undefined; |
Comment on lines
10
to
12
| (supportNpmCiRegistryTesting ? describe : describe.skip)( | ||
| 'package manager rc file is read from the workspace directory when installation is in a capsule', | ||
| 'workspace .yarnrc.yml is read by Yarn when installation is in a capsule', | ||
| function () { |
# Conflicts: # pnpm-lock.yaml
Bump all @pnpm/* deps to their latest 11xx releases. config.reader stays pinned at 1101.3.1 (bit.cloud mirror caps config.env-replace at 3.0.2); add overrides for util.lex-comparator (3.0.2) and config.nerf-dart (1.0.1), whose latest 11xx consumers require ^4/^2 but whose implementations are byte-identical across the major bump and are missing from the bit.cloud mirror. Fix type/runtime fallout from the upgrade in lynx.ts: - pass frozenStore: false to createStoreController (now required) - translate configByUri to the scope-keyed shape (creds under DEFAULT_REGISTRY_SCOPE) that the upgraded @pnpm/network.auth-header expects
Code Review by Qodo
1. ESM import in CJS
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
428
to
434
| if (dangerouslyAllowAllScripts) { | ||
| if (resolvedNeverBuilt == null) { | ||
| // If neverBuiltDependencies is not explicitly set, use a default list | ||
| // we tell pnpm to allow all scripts to be executed, except the packages listed below. | ||
| resolvedNeverBuilt = ['core-js']; | ||
| // pnpm v11's createAllowBuildFunction ignores allowBuilds when dangerouslyAllowAllBuilds is | ||
| // set, so emit allowBuilds with just the deny entries whenever a deny list exists. | ||
| if (!neverBuiltDependencies?.length) { | ||
| allowBuilds['core-js'] = false; | ||
| return { dangerouslyAllowAllBuilds: true, allowBuilds }; | ||
| } |
There was a problem hiding this comment.
1. Core-js deny ignored 🐞 Bug ⛨ Security
In resolveScriptPolicies(), the branch that sets a default deny for "core-js" also returns dangerouslyAllowAllBuilds: true even though the code comment states pnpm ignores allowBuilds when dangerouslyAllowAllBuilds is set. This makes the default deny ineffective and can unintentionally allow core-js build scripts to run when dangerouslyAllowAllScripts is enabled without an explicit neverBuiltDependencies list.
Agent Prompt
### Issue description
`resolveScriptPolicies()` returns `{ dangerouslyAllowAllBuilds: true, allowBuilds }` in the same branch where it sets `allowBuilds['core-js'] = false`, but the adjacent comment states that pnpm ignores `allowBuilds` when `dangerouslyAllowAllBuilds` is set. This makes the default deny entry ineffective.
### Issue Context
The intent (based on existing policy and comments) is to keep a safe default deny list (at least `core-js`) even when `dangerouslyAllowAllScripts` is enabled.
### Fix Focus Areas
- scopes/dependencies/pnpm/lynx.ts[422-439]
### Suggested fix
Adjust the returned options so that the deny entry is actually honored:
- Do **not** set `dangerouslyAllowAllBuilds` in the code path where you need `allowBuilds` to enforce denies (e.g. for `core-js`).
- Alternatively, if pnpm v11 provides a supported way to express “allow all except these denies”, use that API and add/adjust a regression test for the default `core-js` behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The latest 11xx @pnpm packages migrated to a new content-addressable store generation (store.index 1100.2.0 / store.cafs 1100.1.10 / worker 1100.2.0, pulled transitively via the 1102.x store/resolver/fetcher packages) that produces corrupt CAFS file paths on Linux CI ('path must be ... without null bytes' from @pnpm/worker readPkgFromCafs). It passes locally on macOS but breaks e2e. Cap each top-level @pnpm dep at the latest version whose full transitive closure stays on the working store generation (store.index 1100.1.0 / store.cafs 1100.1.9 / worker 1100.1.11): building.commands 1100.1.4, deps.inspection.list 1100.0.17, deps.inspection.tree-builder 1100.0.14, installing.client 1100.2.7, installing.deps-installer 1101.9.0, releasing.commands 1100.4.4, resolving.npm-resolver 1101.5.2, store.connection-manager 1100.2.8, store.controller 1101.0.13, worker 1100.1.11, workspace.projects-graph 1100.0.17. Revert the frozenStore + scope-keyed-auth changes in lynx.ts: on this generation network.auth-header (1101.1.1) reads legacy registryConfig.creds and CreateStoreControllerOptions has no frozenStore field.
|
Code review by qodo was updated up to the latest commit 03d4bc6 |
| import semver from 'semver'; | ||
| import type { LockfilePackageInfo } from '@pnpm/lockfile.types'; | ||
| import * as dp from '@pnpm/dependency-path'; | ||
| import * as dp from '@pnpm/deps.path'; |
There was a problem hiding this comment.
1. Esm import in cjs 🐞 Bug ☼ Reliability
DependenciesGraph now statically imports @pnpm/deps.path, which this PR itself documents as ESM-only; when Babel transpiles this file to CommonJS, it will become a require() and can crash at runtime with ERR_REQUIRE_ESM. This can break dependency graph operations anywhere @teambit/objects is loaded in the CJS-transpiled execution path (e.g. mocha via babel-register).
Agent Prompt
### Issue description
`scopes/scope/objects/models/dependencies-graph.ts` now statically imports `@pnpm/deps.path`. The PR also states `@pnpm/deps.path` is ESM-only, while the repo’s Babel setup transpiles TS modules to CommonJS (turning `import` into `require()`), which can throw `ERR_REQUIRE_ESM` at runtime.
### Issue Context
- This repo uses `@babel/plugin-transform-modules-commonjs` (via `babel-register`) in at least some execution paths.
- The PR already introduced a CJS shim + dynamic `import()` loader for ESM-only pnpm modules elsewhere, which suggests the same problem applies here.
### Fix Focus Areas
- scopes/scope/objects/models/dependencies-graph.ts[1-120]
- scopes/dependencies/pnpm/load-pnpm-esm.cjs[1-12]
- babel.config.js[1-31]
- babel-register.js[1-1]
### Suggested fix directions
Pick one:
1) **Remove the dependency on `@pnpm/deps.path` here** by implementing a small local parser for the *specific* need (extracting `version` from a depPath string) so this code stays synchronous and CJS-safe.
2) **Mirror the shim approach** used in `lockfile-deps-graph-converter.ts`: add a local loader wrapper for `@pnpm/deps.path` and ensure it is initialized before any `DependenciesGraph` code path that calls `dp.parse()`. If you choose this route, add a clear runtime guard/error message when not initialized (instead of an opaque TypeError).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
# Conflicts: # pnpm-lock.yaml
|
Code review by qodo was updated up to the latest commit 93dcf91 |
Code Review by QodoSorry, something went wrongWe weren't able to complete the code review on our side. Please try again |
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
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.
Summary
@pnpm/*deps inworkspace.jsoncto v11's@pnpm/<domain>.<leaf>naming (e.g.@pnpm/core→@pnpm/installing.deps-installer,@pnpm/config→@pnpm/config.reader), bump in-monorepo packages to1100.0.0.mutateModulesnow usesallowBuilds: Record<string, boolean | string>(plusdangerouslyAllowAllBuilds) in place of the removedneverBuiltDependencies/onlyBuiltDependencies/ignoredBuiltDependenciestrio.Config.rawConfig→Config.authConfigrename; read network settings (maxSockets,fetchRetries, etc.) from typedConfigfields instead of rawConfig ini lookups.configByUri: Record<string, RegistryConfig>shape forCreateStoreControllerOptions; switchgenerateResolverAndFetchertocreateResolversince both callers only use.resolve(avoids the now-requiredstoreIndexoncreateClient).sortPackages→sortProjects,createPkgGraph→createProjectsGraph,createOrConnectStoreController→createStoreController; supply now-requiredglobalVirtualStoreDirtogetPeerDependencyIssues.Test plan
npm run check-typespassesbit compile— 310/310 components compileinstall new dependencies (using pnpm)e2e — 5/5 passinstall missing dependencies (when all packages exist)e2e — 2/2 pass (covers add/tag/export/import/reinstall)skipping compilation on installe2e — 2/2 pass