Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct check for existing replacements #109

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JakobJingleheimer
Copy link
Contributor

@JakobJingleheimer JakobJingleheimer commented Mar 21, 2024

quibbleLoaderState.quibbledModules is initialised as a Map in the root of the module, and it is never voided (only replaced with a fresh Map on "reset"), so the empty Map would never fail a falsy test because an empty Map is still truthy (so instead check whether it's emtpy).

To verify, run against JakobJingleheimer/td-global-mock@0008fb5 (npm test will error if this fix doesn't work).

This partially fixes testdouble/testdouble.js#528, but it doesn't address all cases.

@JakobJingleheimer
Copy link
Contributor Author

The test-case "support importing esm and returning the path even when bare-specifier quibbled" is failing locally (but apparently not in CI??), but I'm not sure why—I added some logs, and their output is what I expect.

log output

Note that I manually replaced irrelevantly long parts of URLs below with ...s (the actual output does not contain ...s)

[QBL]:
resolved: file:///.../is-promise/index.mjs
size: 0 (there are no replacements registered)

[QBL]:
resolved: file:///.../is-promise/index.mjs?__quibble=1
size: 1

[QBL]:
resolved: file:///.../is-promise/index.mjs?__quibble=1
size: 1

[QBL]:
resolved: file:///.../is-promise/index.mjs?__quibble=1
size: 1

It fails with (I prettied up the output):

AssertionError:
'file:///.../is-promise/index.mjs?__quibble=1'
to deep equal
'file:///.../is-promise/index.mjs'

Based on the description, I think the expected value is wrong—the description seems to say the result should be "quibbled".

@JakobJingleheimer
Copy link
Contributor Author

@giltayar 👀

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.

td.replaceEsm does not work when specifier is resolved via loader
1 participant