Skip to content

[v3] feat: npm registries in the build script, and a generated .npmrc - #50

Merged
Drownek merged 8 commits into
Drownek:v3-devfrom
monikon22:pr/4-npm-registries
Aug 24, 2026
Merged

[v3] feat: npm registries in the build script, and a generated .npmrc#50
Drownek merged 8 commits into
Drownek:v3-devfrom
monikon22:pr/4-npm-registries

Conversation

@monikon22

Copy link
Copy Markdown
Contributor

Fourth in the series (#46), on top of #49.

Rebased onto v3-dev at 8e1ad36. This branch predates #49's clean rebase, so it still carried a stale copy of the workspace-layout commits underneath its own; those are dropped here and only the six commits actually about registries remain. One conflict, in PlugwrightCompileTestsTask.kt: this branch's own diff still imported JsonReader, which #49's tsconfig fix removed as unused once isLenient went away. Resolved by keeping it dropped — NpmConfig (this branch's own new import) kept. Kotlin compiles.

npm { }

plugwright {
    npm {
        registry("https://registry.internal.example.com", token = secret("NPM_TOKEN"))
        scope("@myorg", "https://registry.internal.example.com", token = secret("NPM_TOKEN"))
    }
}

Credentials are SecretRefs only — a literal token in the build script ends up in the configuration cache and in version control. NpmrcWriter resolves them at execution time and turns the block into the workspace's .npmrc, replacing only a file carrying the marker on its first line that identifies one it wrote itself, so a hand-maintained .npmrc is left alone. A malformed block — a scope without its @, a registry that isn't http(s) — is reported with the rest of the configuration problems rather than by npm 404ing against the public registry several minutes later.

Both plugwrightInit and plugwrightCompileTests write the file before their own npm install, so the scaffold and the dependency/runner-package installs that follow all see the same registries. .npmrc is gitignored, in both the init template and the example workspace — it may hold a token.

Two fixes that ride along

Windows runs npm through cmd /c, where ^ is the escape character — @scope/pkg@^1.2.0 was reaching npm as @scope/pkg@1.2.0, an exact version nobody published, reported as ETARGET. Fixed at the point Java quotes the argument.

Runner packages are also deduplicated by name now. A mode names the package it needs a version of, and the build script can name it again, bare, in plugins { npm(...) }; both specs in one install was npm resolving the same package twice, and the bare one asks for a "latest" a package released under another tag doesn't have.

Docs

configuration.mdx gets the reference for npm { }: the registry/scope calls, credentials as SecretRefs, what the generated .npmrc looks like, and when plugwright refuses to touch one. ci-cd.mdx gets the token-through-the-environment version, since that's where private registries actually bite. project-layout.mdx and quickstart.mdx just mention the file.

Merging

Same request: merge commit or rebase and merge, not squash and merge — PR 5 is stacked on this branch.

An npm { } block on the extension names the registries the workspace installs
from, per scope where it needs to be, plus any other npmrc option. Credentials
are SecretRefs only - a literal token in a build script ends up in the
configuration cache and in version control.

NpmrcWriter turns the block into the workspace .npmrc. It resolves the secrets
at execution time and only ever replaces a file it wrote itself, which the
marker on the first line identifies. Nothing calls it yet.
plugwrightCompileTests writes the file just before npm install, so both the
dependency install and the runner packages that follow it - same working
directory, same npm - fetch from the configured registries.

A malformed block (a scope without its @, a registry that is not http) is
reported with the rest of the configuration problems rather than by npm 404ing
against the public registry several minutes later.
plugwrightInit writes the file before its own npm install: a scaffold that can
only reach the public registry is no use to a project that lives behind a
private one.

The .gitignore template lists .npmrc, and a workspace created before this got
one keeps its own file and gains the entry the first time the .npmrc is
generated. It may hold a registry token.
Configuration gets the reference for the npm { } block: the registry and scope
calls, credentials as SecretRefs, what the generated .npmrc looks like and when
plugwright refuses to touch one. CI/CD gets the token-through-the-environment
version, since that is where private registries actually bite.

Project layout and quickstart just mention the file, which is gitignored and
regenerated per install.
Windows runs npm through `cmd /c`, where `^` is the escape character, so
`@scope/pkg@^1.2.0` reached npm as `@scope/pkg@1.2.0` — an exact version
nobody published, reported as ETARGET. Java quotes an argument only for a
space or a redirection, so the quoting has to happen here.

Runner packages are also deduplicated by package name now. A mode names the
package it needs a version of, and the build script names it again, bare, in
plugins { npm(...) }; both specs in one install is npm resolving the same
package twice, and the bare one asks for a "latest" that a package released
under another tag does not have.
@Drownek

Drownek commented Aug 24, 2026

Copy link
Copy Markdown
Owner

The PR looks very good overall! The fix for the ^ issue on Windows is smart, and the package deduplication logic is correct.

However, I noticed two edge cases that might cause issues:

1. URL parsing in npmPackageNameOf:
This method assumes that the last @ is always the version separator. But npm also allows URLs as packages. If a user provides a Git URL with SSH (git+ssh://git@github.com/user/repo) or an HTTP URL with credentials (https://user:pass@registry...), the method will wrongly cut the URL at the last @. If a user adds two different Git packages, they will be saved under the same broken name in the map, and one will overwrite the other.
Suggested fix: Before looking for @, add a check to return early if the string is a URL (for example, if (spec.contains("://") || spec.startsWith("git+")) return spec).

2. Windows cmd /c quotes bug:
The new quoteForCmd method adds quotes to arguments to protect them. But Windows cmd.exe has a weird bug: if the command after /c starts with a quote (like "C:\Program Files\nodejs\npm.cmd" — which Java adds automatically if the path has spaces) AND the whole command has more than 2 quotes (which will happen now because arguments also have quotes), cmd /c will automatically remove the first and last quote of the entire command. This will break the path to npm.cmd and the build will fail.
Suggested fix: A classic workaround in Java is to add the call command. In runCommand, instead of listOf("cmd", "/c") + ..., you can use listOf("cmd", "/c", "call") + .... Because the command will start with the letter c (from call) instead of a quote ", cmd will not remove any quotes.

Everything else, including the .gitignore update and the .npmrc logic, looks great!

npmPackageNameOf assumed the last @ was always the version
separator. git+ssh://git@host/repo and https://user:pass@host/pkg
carry @ of their own, so two different URL specs got truncated to
the same wrong key and collided in the map.

Reported by Drownek on PR Drownek#50.
cmd.exe strips the outer pair of quotes from the whole command line
when the line starts with a quote and holds more than two quotes
total. quoteForCmd now adds quotes to protect ^ in version ranges,
so a spaced npm.cmd path (already quoted by Java) plus one quoted
argument hits that case and cmd mangles the path.

call makes the line start with a letter instead of a quote, which
cmd doesn't touch.

Reported by Drownek on PR Drownek#50.
@monikon22

Copy link
Copy Markdown
Contributor Author

Good catches, both fixed.

  1. npmPackageNameOf now returns the spec as-is when it's a URL (:// or git+ prefix) before touching the trailing-@ logic — SSH git URLs and credentialed HTTPS URLs no longer collide in the map.
  2. runCommand now runs cmd /c call ... instead of cmd /c ..., so the line handed to cmd starts with call and not a quote — cmd no longer strips the outer quote pair once quoteForCmd adds its own.

Pushed to the branch.

@Drownek
Drownek merged commit 03e8df6 into Drownek:v3-dev Aug 24, 2026
Drownek added a commit that referenced this pull request Aug 24, 2026
[v3] feat: npm registries in the build script, and a generated .npmrc
@Drownek

Drownek commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Sorry, I misclicked and squash-merged this by accident! To avoid messing up the git history for your upcoming dependent PRs, I quickly undid it and manually created a proper merge commit via CLI. Everything is fixed and next PRs are safe

monikon22 added a commit to monikon22/plugwright that referenced this pull request Aug 24, 2026
npmPackageNameOf assumed the last @ was always the version
separator. git+ssh://git@host/repo and https://user:pass@host/pkg
carry @ of their own, so two different URL specs got truncated to
the same wrong key and collided in the map.

Reported by Drownek on PR Drownek#50.
monikon22 added a commit to monikon22/plugwright that referenced this pull request Aug 24, 2026
cmd.exe strips the outer pair of quotes from the whole command line
when the line starts with a quote and holds more than two quotes
total. quoteForCmd now adds quotes to protect ^ in version ranges,
so a spaced npm.cmd path (already quoted by Java) plus one quoted
argument hits that case and cmd mangles the path.

call makes the line start with a letter instead of a quote, which
cmd doesn't touch.

Reported by Drownek on PR Drownek#50.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants