Repository navigation
Fix the pnpm workspace file I broke CI with - #270
Merged
Merged
Conversation
My fault, from #267. The pnpm-workspace.yaml I added to approve esbuild's build script broke pnpm install in CI: ERROR packages field missing or empty pnpm 9, which the workflow pins, treats any pnpm-workspace.yaml as a workspace declaration and refuses to install without a packages field. pnpm 12, which I have locally, does not. I tested the install on my machine, saw it succeed, and shipped a file that only works on the version I happened to have. The file now carries both keys, one for each version: packages: ["."] so pnpm 9 is satisfied and the layout is unchanged, and ignoredBuiltDependencies so pnpm 10+ stops refusing an install that skipped a build script. Skipping it is deliberate. esbuild's script places a prebuilt binary that neither the build nor the tests need, verified by wiping node_modules, installing without it, and running pnpm build and pnpm test. The approval in #267 was me reacting to a warning without checking whether it meant anything. The workflow also runs the frontend tests now. #267 added them and never wired them in, so the job that failed would not have caught a broken test either.
The frontend tests I wired in one commit ago fail on the
runner:
TypeError: webidl.util.markAsUncloneable is not a
function
at new CacheStorage [email protected]/.../cachestorage.js
at [email protected]/lib/api.js
jsdom 30, which vitest pulls in, declares engines
^22.22.2 || ^24.15.0 || >=26.0.0, and undici 8 under it
wants >=22.19.0. CI ran Node 20, where the import dies
with a message that names neither Node nor a version.
Both workflows move to 22. The runners had already
annotated the previous run to say 20 is deprecated and
that actions are being forced onto 24, so this was due
anyway.
frontend/package.json gains an engines field so the
requirement is visible where someone would look for it,
rather than only in a transitive dependency's metadata.
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
1 participant
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.
My fault, from #267. The
pnpm-workspace.yamlI added to approve esbuild's build script brokepnpm installin CI:pnpm 9, which the workflow pins, treats any
pnpm-workspace.yamlas a workspace declaration and refuses to install without apackagesfield. pnpm 12, which I have locally, does not. I tested the install on my machine, saw it succeed, and shipped a file that only works on the version I happened to have.The file now carries both keys, one for each version, with a comment saying which is which:
packages: ["."]— this package itself, so pnpm 9 is satisfied and the install layout is unchanged.ignoredBuiltDependencies: [esbuild]— pnpm 10+ refuses to finish an install that silently skipped a build script (ERR_PNPM_IGNORED_BUILDS) unless told the skip is deliberate.And it is deliberate: esbuild's script places a prebuilt binary that neither the build nor the tests need. Verified by wiping
node_modules, installing without the script, and runningpnpm buildandpnpm test— both green. The original approval in #267 was me reacting to a warning without checking whether it meant anything.The workflow also now runs the frontend tests. #267 added them and I never wired them into CI, so the job that just failed would not have caught a broken test either.
Verified locally from a wiped
node_modules:pnpm installexits 0, build succeeds, 20 tests pass. I have no pnpm 9 here, so thepackageshalf is reasoned from the exact error CI produced rather than measured; this run will settle it.