Repository navigation
Conversation
td.replaceEsm('some-package') resolves the package through quibble, which
under pnpm can't see packages that only the app depends on unless pnpm
hoisting happens to make them visible (testdouble/quibble#84). The new
examples/node-esm-pnpm project is a pnpm workspace with hoisting off whose
packages/app test replaces is-number, a dependency of that package only.
It has to be a workspace: in a single pnpm project the app's own
node_modules is still within reach of quibble's.
It fails with quibble 0.11.x (the real is-number runs, because quibble
replaces a different copy) and with 0.12.0 on Node 24+ (the generated stub
for a CommonJS module is a SyntaxError), so it's expected to fail until
quibble is bumped to 0.12.1.
script/run-examples installs examples that have a pnpm-workspace.yaml with
pnpm, and CI installs pnpm (8 on Node 16, 9 on Node 18, latest otherwise).
The example sets hoisting off in both pnpm-workspace.yaml (pnpm 10+) and
.npmrc (pnpm 8/9), and minimumReleaseAge: 0 so pnpm doesn't add exclusions
to pnpm-workspace.yaml for just-released dependencies. Its lockfile isn't
committed, so run-examples installs with --no-frozen-lockfile.
Picks up two fixes for replacing ES modules:
- 0.12.0 resolves bare specifiers from the caller instead of from quibble's
own directory, so td.replaceEsm('some-package') replaces the copy the
test imports under pnpm, and in monorepos where packages need different
versions of a dependency (testdouble/quibble#84)
- 0.12.1 generates valid stubs for export names that aren't identifiers,
so td.replaceEsm() of a CommonJS package no longer fails on Node 24+ with
"SyntaxError: Unexpected token '.'" (Node gives CommonJS modules a
'module.exports' named export)
With these, the node-esm-pnpm example passes.
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.
td.replaceEsm()didn't work reliably under pnpm when given a package name. This adds an example project that reproduces the bug with pnpm in CI, and bumps quibble to 0.12.1, which fixes it (along with a related Node 24+ bug the example also hit).Why it broke
td.replaceEsm('is-number')asks quibble for the package's exact file URL, then mocks that URL. quibble resolved the package name as if quibble itself had imported it, not the test file. Under npm's flatnode_modules, both see the same packages. Under pnpm, quibble lives in its own isolated directory. It either couldn't find the package (Cannot find package 'is-number') or found a different copy and mocked that one, leaving the real module in place. quibble 0.12.0 fixes this by resolving package names from the calling file (testdouble/quibble#123, testdouble/quibble#84).When
td.replaceEsm()isn't given a replacement, it imitates the real module and passes all of its export names to quibble. On Node 24+, a CommonJS package likeis-numberhas an export named'module.exports'. quibble's generated stub declared it asexport let module.exports = …, which is a syntax error. quibble 0.12.1 exports stubs under string names instead, so any name works (testdouble/quibble#125).Why the example catches it
examples/node-esm-pnpmis a small pnpm workspace with hoisting turned off and one package,packages/app. Its test replacesis-number, which onlypackages/appdepends on, and checks the code under test sees the replacement.It has to be a workspace. In a single pnpm project, the app's own
node_modulesis still within reach of quibble's, so the bug never shows. With the test in a nested package, testdouble.js and quibble are installed in the workspace root, whereis-numberisn't visible.Results for the example, by quibble version:
is-numberruns (wrong copy replaced)SyntaxError: Unexpected token '.'What this PR changes
examples/node-esm-pnpm: the new example. Hoisting is off in bothpnpm-workspace.yaml(pnpm 10+) and.npmrc(pnpm 8 and 9).minimumReleaseAge: 0stops pnpm from editingpnpm-workspace.yamlwhen testdouble.js's dependencies were just released. Its lockfile isn't committed.script/run-examples: examples with apnpm-workspace.yamlare installed withpnpm install --no-frozen-lockfile, and it stops with an install hint if pnpm is missing.^0.12.1, along with the example lockfiles.The commits are in that order on purpose: the example is added first and fails until the quibble bump in the second commit.
Running
npm testlocally now requires pnpm to be installed.How it was tested
Follow-up: quibble's advisory testdouble.js integration workflow runs this suite without pnpm installed, so it will need the same pnpm step.