Skip to content
Open
Show file tree
Hide file tree
Changes from 11 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
11,828 changes: 5,077 additions & 6,751 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