Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = function (api) {
},
useBuiltIns: 'usage',
corejs: 3,
exclude: ['@babel/plugin-transform-dynamic-import'],
},
],
];
Expand Down
2 changes: 1 addition & 1 deletion components/legacy/e2e-helper/npm-ci-registry.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint no-console: 0 */
import { addUser, REGISTRY_MOCK_PORT, start as startRegistryMock, prepare } from '@pnpm/registry-mock';
import type { ChildProcess } from 'child_process';
import { fetch } from '@pnpm/fetch';
import fs from 'fs-extra';
import execa from 'execa';
import * as path from 'path';
Expand Down Expand Up @@ -79,6 +78,7 @@ export class NpmCiRegistry {
resolved = true;
let fetchResults;
try {
const { fetch } = await import('@pnpm/network.fetch');
fetchResults = await fetch(`http://localhost:${REGISTRY_MOCK_PORT}/is-odd`, {
retry: {
minTimeout: 1000,
Expand Down
8 changes: 6 additions & 2 deletions e2e/harmony/dependency-resolver.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import chai, { expect } from 'chai';
import path from 'path';
import fs from 'fs-extra';
import type { Modules } from '@pnpm/modules-yaml';
import { readModulesManifest } from '@pnpm/modules-yaml';
import type { Modules } from '@pnpm/installing.modules-yaml';
import { generateRandomStr } from '@teambit/toolbox.string.random';
import rimraf from 'rimraf';
import { Extensions } from '@teambit/legacy.constants';
Expand All @@ -13,6 +12,11 @@ chai.use(chaiFs);

chai.use(assertArrays);

async function readModulesManifest(modulesDir: string): Promise<Modules | null> {
const m = await import('@pnpm/installing.modules-yaml');
return m.readModulesManifest(modulesDir);
}

describe('dependency-resolver extension', function () {
let helper: Helper;
this.timeout(0);
Expand Down
30 changes: 3 additions & 27 deletions e2e/harmony/pkg-manager-config.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import path from 'path';
import chai, { expect } from 'chai';
import { readModulesManifest } from '@pnpm/modules-yaml';
import chaiString from 'chai-string';

import { Helper, NpmCiRegistry, supportNpmCiRegistryTesting } from '@teambit/legacy.e2e-helper';
import chaiFs from 'chai-fs';
import { Helper, NpmCiRegistry, supportNpmCiRegistryTesting } from '@teambit/legacy.e2e-helper';

chai.use(chaiFs);
chai.use(chaiString);

(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 10 to 12
Comment on lines 10 to 12
Comment on lines 10 to 12
this.timeout(0);
Comment on lines 10 to 13
let helper: Helper;
Expand Down Expand Up @@ -66,29 +65,6 @@ chai.use(chaiString);
expect(isPositivePath).to.be.a.path();
});
});
describe('using pnpm', () => {
before(() => {
helper.scopeHelper.reInitWorkspace({
npmrcConfig: {
'hoist-pattern[]': 'foo',
},
});
helper.extensions.workspaceJsonc.addKeyValToDependencyResolver('packageManager', `teambit.dependencies/pnpm`);
helper.scopeHelper.addRemoteScope();
helper.workspaceJsonc.setupDefault();
helper.fixtures.populateComponents(1);
helper.extensions.addExtensionToVariant('comp1', `${envId1}@0.0.1`);
helper.capsules.removeScopeAspectCapsules();
helper.command.status(); // populate capsules.
});
it('workspace .npmrc is taken into account when running install in the capsule', async () => {
const { scopeAspectsCapsulesRootDir } = helper.command.capsuleListParsed();
const modulesState = await readModulesManifest(
path.join(scopeAspectsCapsulesRootDir, `${helper.scopes.remote}_node-env-1@0.0.1/node_modules`)
);
expect(modulesState?.hoistPattern?.[0]).to.eq('foo');
});
});
after(() => {
npmCiRegistry.destroy();
});
Comment on lines 68 to 70
Expand Down
8 changes: 6 additions & 2 deletions e2e/harmony/pnpm-default-hoisting.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { expect } from 'chai';
import path from 'path';
import type { Modules } from '@pnpm/modules-yaml';
import { readModulesManifest } from '@pnpm/modules-yaml';
import type { Modules } from '@pnpm/installing.modules-yaml';
import { Helper } from '@teambit/legacy.e2e-helper';

async function readModulesManifest(modulesDir: string): Promise<Modules | null> {
const m = await import('@pnpm/installing.modules-yaml');
return m.readModulesManifest(modulesDir);
}

describe('pnpm install with default settings', function () {
let helper: Helper;
let modulesState: Modules | null;
Expand Down
8,866 changes: 3,688 additions & 5,178 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion scopes/component/modules/merge-helper/merge-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,5 @@ export async function mergeFiles({
${chalk.bold('command:')} ${err.command}
${chalk.bold('message:')} ${err.message}
${chalk.bold('original error:')} ${err.stderr}`);
throw err;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import fs from 'fs-extra';
import { assign, parse } from 'comment-json';
import { ComponentID } from '@teambit/component-id';
import { readCAFileSync } from '@pnpm/network.ca-file';
import { parseBareSpecifier } from '@pnpm/npm-resolver';
import { parseBareSpecifier } from '@pnpm/napi';
import type { SourceFile } from '@teambit/component.sources';
import type { ProjectManifest, DependencyManifest } from '@pnpm/types';
import semver, { SemVer } from 'semver';
Expand Down Expand Up @@ -1474,12 +1474,11 @@ as an alternative, you can use "+" to keep the same version installed in the wor
* E.g.: https://registry.npmjs.org/is-odd/-/is-odd-0.1.0.tgz)
*/
isValidVersionSpecifier(spec: string): boolean {
// parseBareSpecifier returns null for empty/unparsable input.
return (
parseBareSpecifier(
spec,
'pkgname', // This argument is the package but we don't need it
'latest',
'https://registry.npmjs.org/'
'pkgname' // This argument is the package name but we don't need it
) != null
Comment on lines 1476 to 1482

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Eager napi import 🐞 Bug ☼ Reliability

dependency-resolver.main.runtime.ts now imports parseBareSpecifier from @pnpm/napi at module scope,
forcing the native binding to load whenever the dependency-resolver aspect is loaded. This can
increase startup cost and can hard-fail process initialization in environments where the native
binding cannot be loaded, even if pnpm isn’t the active package manager.
Agent Prompt
### Issue description
`dependency-resolver.main.runtime.ts` performs a module-scope import from `@pnpm/napi` and uses it in `isValidVersionSpecifier()`. This eagerly loads the native binding whenever the dependency-resolver aspect is loaded, which can slow startup and can crash early if the native module can’t be loaded.

### Issue Context
Other pnpm integration points in this repo explicitly lazy-load pnpm internals for performance, so this eager import is inconsistent and increases blast radius.

### Fix Focus Areas
- Replace the module-scope `@pnpm/napi` import with a non-native, lightweight validator (e.g. `semver.valid/validRange` + allow-list for tags/aliases/URLs), OR
- If `parseBareSpecifier` is required for parity, load it lazily inside `isValidVersionSpecifier()` with a guarded `require()`/cached getter and provide a safe fallback when the binding fails to load.

- scopes/dependencies/dependency-resolver/dependency-resolver.main.runtime.ts[36-46]
- scopes/dependencies/dependency-resolver/dependency-resolver.main.runtime.ts[1469-1484]
- scopes/dependencies/pnpm/pnpm.package-manager.ts[127-134]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

);
}
Expand Down
2 changes: 1 addition & 1 deletion scopes/dependencies/dependency-resolver/package-manager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PeerDependencyIssuesByProjects } from '@pnpm/core';
import type { PeerDependencyIssuesByProjects } from '@pnpm/napi';
import type { PeerDependencyRules, ProjectManifest, DependencyManifest } from '@pnpm/types';
import type { ComponentID, ComponentMap, Component } from '@teambit/component';
import { type DependenciesGraph } from '@teambit/objects';
Expand Down
4 changes: 2 additions & 2 deletions scopes/dependencies/pnpm/get-proxy-config.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import type { PackageManagerProxyConfig } from '@teambit/dependency-resolver';
import type { Config } from '@pnpm/config';
import type { Config } from '@pnpm/config.reader';

export function getProxyConfig(config: Config): PackageManagerProxyConfig {
const httpProxy = config.httpProxy;
const httpsProxy = config.httpsProxy || httpProxy;
const proxyConfig: PackageManagerProxyConfig = {
httpProxy,
httpsProxy,
noProxy: config.rawConfig.noproxy,
noProxy: config.authConfig.noproxy,
};
Comment on lines 4 to 11

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Noproxy from authconfig 🐞 Bug ≡ Correctness

getProxyConfig() sets noProxy from config.authConfig.noproxy, which is not the typed
proxy-bypass setting and may not contain the user’s .npmrc no-proxy value. As a result, dependency
downloads can incorrectly go through the proxy even when the user configured domains to bypass it.
Agent Prompt
### Issue description
`getProxyConfig()` sources `noProxy` from `config.authConfig.noproxy`. `authConfig` is used primarily for auth/credential configuration, and using it for proxy-bypass settings risks ignoring `.npmrc` `no-proxy`/`noproxy` configuration.

### Issue Context
Bit’s dependency-resolver supports `noProxy` explicitly and threads it through proxy configuration; if `getProxyConfig()` fails to populate it, proxy bypass won’t work.

### Fix Focus Areas
- Prefer the typed config field for proxy bypass if available (e.g. `config.noProxy`).
- If pnpm’s config reader still exposes raw keys, add a safe fallback chain (e.g. `config.noProxy ?? config.authConfig['no-proxy'] ?? config.authConfig.noproxy`).

- scopes/dependencies/pnpm/get-proxy-config.ts[1-13]
- scopes/dependencies/dependency-resolver/dependency-resolver-workspace-config.ts[55-59]
- scopes/dependencies/dependency-resolver/dependency-resolver.main.runtime.ts[805-809]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

return proxyConfig;
}
Expand Down
10 changes: 5 additions & 5 deletions scopes/dependencies/pnpm/get-registries.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import getCredentialsByURI from 'credentials-by-uri';
import type { RegistriesMap } from '@teambit/dependency-resolver';
import { stripTrailingChar } from '@teambit/toolbox.string.strip-trailing-char';
import type { Config } from '@pnpm/config';
import type { Config } from '@pnpm/config.reader';
import { isEmpty } from 'lodash';
import toNerfDart from 'nerf-dart';

Expand All @@ -15,11 +15,11 @@ export function getRegistries(config: Config): RegistriesMap {

Object.keys(config.registries).forEach((regName) => {
const uri = config.registries[regName];
let credentials = getCredentialsByURI(config.rawConfig, uri);
let originalAuthConfig = getOriginalAuthConfigByUri(config.rawConfig, uri);
let credentials = getCredentialsByURI(config.authConfig, uri);
let originalAuthConfig = getOriginalAuthConfigByUri(config.authConfig, uri);
if (isEmpty(credentials)) {
credentials = getCredentialsByURI(config.rawConfig, switchTrailingSlash(uri));
originalAuthConfig = getOriginalAuthConfigByUri(config.rawConfig, switchTrailingSlash(uri));
credentials = getCredentialsByURI(config.authConfig, switchTrailingSlash(uri));
originalAuthConfig = getOriginalAuthConfigByUri(config.authConfig, switchTrailingSlash(uri));
Comment on lines +18 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Auth registry key missing 🐞 Bug ≡ Correctness

getRegistries() now passes config.authConfig into getOriginalAuthConfigByUri(), but that
helper reads config.registry to compute the default registry nerf-dart; if authConfig doesn’t
carry a registry string, the default-auth fallback logic can crash or behave incorrectly.
Agent Prompt
### Issue description
`getRegistries()` calls `getOriginalAuthConfigByUri(config.authConfig, uri)`, but `getOriginalAuthConfigByUri()` expects the passed object to contain `registry` (used as the default registry URI). With `@pnpm/config.reader`, `Config.registries` is already the authoritative source of registry URLs, so relying on `authConfig.registry` is brittle and may be `undefined`.

### Issue Context
This impacts building `Registry` objects (default and scoped) used across installs/resolution.

### Fix Focus Areas
- scopes/dependencies/pnpm/get-registries.ts[13-46]

### Suggested fix
Option A (preferred, explicit):
1) Change `getOriginalAuthConfigByUri` signature to accept `defaultRegistryUri: string`.
2) Call it as `getOriginalAuthConfigByUri(config.authConfig, uri, config.registries.default)` (or equivalent default-registry entry).
3) Replace `toNerfDart(config.registry)` with `toNerfDart(defaultRegistryUri)`.

Option B (minimal change):
- Call `getOriginalAuthConfigByUri({ ...config.authConfig, registry: config.registries.default }, uri)` to ensure the expected `registry` key exists.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
registriesMap[regName] = {
uri,
Expand Down
12 changes: 12 additions & 0 deletions scopes/dependencies/pnpm/load-pnpm-esm.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

// Native CJS so Babel's modules-commonjs transform leaves the import() alone.
// @pnpm/deps.path and @pnpm/lockfile.fs are ESM-only and must go through
// Node's ESM loader.
exports.loadEsm = async () => {
const [dp, lockfileFs] = await Promise.all([
import('@pnpm/deps.path'),
import('@pnpm/lockfile.fs'),
]);
return { dp, getLockfileImporterId: lockfileFs.getLockfileImporterId };
};
29 changes: 20 additions & 9 deletions scopes/dependencies/pnpm/lockfile-deps-graph-converter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import path from 'path';
import { ComponentID } from '@teambit/component';
import { DependenciesGraph, type PackagesMap, type DependencyEdge } from '@teambit/objects';
import { convertLockfileToGraph, convertGraphToLockfile } from './lockfile-deps-graph-converter';
import {
convertLockfileToGraph,
convertGraphToLockfile,
init as initLockfileDepsGraphConverter,
} from './lockfile-deps-graph-converter';
import { type BitLockfileFile } from './lynx';
import { expect } from 'chai';

before(async () => {
await initLockfileDepsGraphConverter();
});

describe('convertLockfileToGraph simple case', () => {
const lockfile: BitLockfileFile = {
bit: {
Expand Down Expand Up @@ -108,14 +116,17 @@ describe('convertLockfileToGraph simple case', () => {
},
},
};
const graph = convertLockfileToGraph(lockfile, {
pkgName: 'comp1',
componentRelativeDir: 'comps/comp1',
componentRootDir: 'node_modules/.bit_roots/env',
componentIdByPkgName: new Map([
['comp1', ComponentID.fromString('my-scope/comp1@1.0.0')],
['comp2', ComponentID.fromString('my-scope/comp2@1.0.0')],
]),
let graph: DependenciesGraph;
before(() => {
graph = convertLockfileToGraph(lockfile, {
pkgName: 'comp1',
componentRelativeDir: 'comps/comp1',
componentRootDir: 'node_modules/.bit_roots/env',
componentIdByPkgName: new Map([
['comp1', ComponentID.fromString('my-scope/comp1@1.0.0')],
['comp2', ComponentID.fromString('my-scope/comp2@1.0.0')],
]),
});
});
const expected = {
schemaVersion: '2.0',
Expand Down
41 changes: 35 additions & 6 deletions scopes/dependencies/pnpm/lockfile-deps-graph-converter.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import path from 'path';
import { type ProjectManifest } from '@pnpm/types';
import { type LockfileFileProjectResolvedDependencies } from '@pnpm/lockfile.types';
import { type ResolveFunction } from '@pnpm/client';
import * as dp from '@pnpm/dependency-path';
import type * as Dp from '@pnpm/deps.path';
import type { getLockfileImporterId as GetLockfileImporterId } from '@pnpm/lockfile.fs';
import { pick, partition } from 'lodash';
import { BitError } from '@teambit/bit-error';
import { snapToSemver } from '@teambit/component-package-version';
Expand All @@ -18,10 +18,39 @@ import {
type CalcDepsGraphForComponentOptions,
type ComponentIdByPkgName,
} from '@teambit/dependency-resolver';
import { getLockfileImporterId } from '@pnpm/lockfile.fs';
import normalizePath from 'normalize-path';
import { type BitLockfileFile } from './lynx';

/**
* Minimal structural signature of the `resolve` function returned by
* `generateResolverAndFetcher` in `./lynx` (backed by `@pnpm/napi`'s
* `resolveDependency`). Only the parts this module consumes are typed.
*/
type ResolveFunction = (
wantedDependency: { alias?: string; bareSpecifier?: string },
opts: { lockfileDir?: string; projectDir?: string; preferredVersions?: Record<string, unknown> }
) => Promise<{ resolution?: Record<string, unknown> }>;

// @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 +34 to +40

export function init(): Promise<void> {
loading ??= (async () => {
const { loadEsm } = require('./load-pnpm-esm.cjs') as {
loadEsm: () => Promise<{ dp: typeof Dp; getLockfileImporterId: typeof GetLockfileImporterId }>;
};
const m = await loadEsm();
dp = m.dp;
getLockfileImporterId = m.getLockfileImporterId;
})();
return loading;
}

function convertLockfileToGraphFromCapsule(
lockfile: BitLockfileFile,
{
Expand Down Expand Up @@ -374,9 +403,9 @@ export async function convertGraphToLockfile(
throw err;
}
const { resolution } = resolveResult;
if ('integrity' in resolution && resolution.integrity) {
if (resolution != null && 'integrity' in resolution && resolution.integrity) {
lockfile.packages[pkgToResolve.pkgId].resolution = {
integrity: resolution.integrity,
integrity: resolution.integrity as string,
};
}
}
Expand Down Expand Up @@ -464,7 +493,7 @@ function getPkgsToResolve(lockfile: BitLockfileFile, manifests: Record<string, P
return pkgsToResolve;
}

function depPathToRef(depPath: dp.DependencyPath): string {
function depPathToRef(depPath: Dp.DependencyPath): string {
return `${depPath.version}${depPath.patchHash ?? ''}${depPath.peerDepGraphHash ?? ''}`;
}

Expand Down
Loading