fix(core): actionable error for rs.spyOn on ESM namespace export#1498
Merged
Conversation
Deploying rstest with
|
| Latest commit: |
6676d3b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d8f9947e.rstest.pages.dev |
| Branch Preview URL: | https://worktree-radiant-mixing-bonb.rstest.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02e1f3a34a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When spying an export of a native ES module namespace (e.g. an externalized
node_modules dependency in the `node` test environment), the binding is
non-configurable, so `rs.spyOn` failed with a bare "Cannot redefine property".
It now throws an actionable error pointing at `rs.mock('<module>', { spy: true })`.
The guard is precise: it fires only for a module namespace (`isModuleObject`)
whose target export is non-configurable, so bundled deps (configurable getters)
and ordinary non-configurable object properties keep their existing behavior.
For a bundled re-export, where the spy silently installs but is bypassed by
re-export inlining, this cannot be detected at runtime — it is documented as a
limitation with `rs.mock(..., { spy: true })` as the recommended alternative.
Docs (en/zh) and regression tests included.
Refs #1492
fi3ework
force-pushed
the
worktree-radiant-mixing-bonbon
branch
from
02e1f3a to
6676d3b
Compare
The guard reused isModuleObject, which also matches an __esModule interop object from a transpiled CommonJS module. Those are ordinary objects whose writable (even if non-configurable) exports tinyspy can still spy, so the guard wrongly threw for them. Narrow it to a real [object Module] namespace (Symbol.toStringTag === 'Module') so __esModule exports fall through and stay spyable.
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.
Summary
When a test spies an export of a native ES module namespace — e.g. an externalized
node_modulesdependency in thenodetest environment, or a re-exported binding — the namespace binding is non-configurable, sors.spyOn(ns, 'method')failed with a bareTypeError: Cannot redefine property.rs.spyOnnow detects that exact shape (a module namespace whose target export is non-configurable, via the existingisModuleObject) and throws an actionable error pointing users atrs.mock('<module>', { spy: true }), which replaces the module's exports at build time and works regardless of re-export optimization.The guard is deliberately narrow: bundled dependencies expose configurable getters (so it never fires for them), and ordinary non-configurable object properties are not module namespaces (so their original error is untouched). For a bundled re-export — where the spy silently installs but re-export inlining routes callers past it — the ineffectiveness cannot be detected at runtime, so it is documented as a limitation (EN/ZH) with
rs.mock(..., { spy: true })as the recommended alternative.Related Links
Checklist