Fix TYPESPEC_NPM_REGISTRY forwarding during install - #11694
Fix TYPESPEC_NPM_REGISTRY forwarding during install#11694Vivek JM (vivekjm) wants to merge 2 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR forwards the TYPESPEC_NPM_REGISTRY override to the npm process spawned by tsp install, ensuring installs use the same registry as manifest resolution.
Changes:
- Add
getNpmRegistryEnvironment()to produce a child-processenvwithnpm_config_registryset fromTYPESPEC_NPM_REGISTRY. - Update installer to use the new environment builder when invoking the package manager.
- Add unit tests covering forwarding behavior and preservation of existing npm registry env vars.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/compiler/test/package-manager/npm-registry.test.ts | Adds tests validating registry env forwarding and non-interference when no override is set. |
| packages/compiler/src/package-manger/npm-registry.ts | Introduces getNpmRegistryEnvironment() to map TypeSpec registry override to npm’s env convention. |
| packages/compiler/src/install/install.ts | Uses getNpmRegistryEnvironment() when spawning the package manager to apply the override. |
| .chronus/changes/forward-typespec-npm-registry-2026-08-15.md | Adds changelog entry documenting the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/compiler/src/package-manger/npm-registry.ts:112
- This reads
process.env[\"TYPESPEC_NPM_REGISTRY\"](trimmed) in the guard, and then readsprocess.envagain indirectly viagetNpmRegistry()when settingnpm_config_registry. To keep behavior consistent and easier to reason about, compute the effective registry once (e.g., a singleconst registry = ...) and use that value for both the guard and the assignment. This also avoids any chance of inconsistency ifprocess.envwere to change between the two reads.
export function getNpmRegistryEnvironment(): NodeJS.ProcessEnv {
const environment = { ...process.env };
if (!process.env["TYPESPEC_NPM_REGISTRY"]?.trim()) {
return environment;
}
// Environment variable names are case-insensitive on Windows. Remove any
// existing spelling so Node does not choose between duplicate keys.
for (const name of Object.keys(environment)) {
if (name.toLowerCase() === "npm_config_registry") {
delete environment[name];
}
}
environment["npm_config_registry"] = getNpmRegistry();
return environment;
|
thank you so much for your contribution Vivek JM (@vivekjm), Timothee Guerin (@timotheeguerin) can you take a look at this when you're back from vacation? Thanks! |
commit: |
|
All changed packages have been documented.
Show changes
|
Summary
TYPESPEC_NPM_REGISTRYto the npm process launched bytsp installFixes #11688.
Root cause
TypeSpec already used
TYPESPEC_NPM_REGISTRYwhen it fetched package-manager metadata and archives. The laternpm installsubprocess inherited that TypeSpec-specific variable, but npm readsnpm_config_registryinstead, so project dependencies still came from npm's default configuration.Testing
pnpm setup:minpnpm --filter @typespec/compiler exec vitest run test/package-manager/npm-registry.test.ts— 6 tests passedpnpm --filter @typespec/compiler run buildpnpm exec prettier --check packages/compiler/src/install/install.ts packages/compiler/src/package-manger/npm-registry.ts packages/compiler/test/package-manager/npm-registry.test.ts .chronus/changes/forward-typespec-npm-registry-2026-08-15.mdpnpm --filter @typespec/compiler run lintgit diff --checkLocal setup note: I initially invoked the compiler tests before building the generated manifest and internal workspace packages, which produced bootstrap-related missing-module errors. After running the documented
pnpm setup:minstep, the focused tests and all checks above passed.