Repository navigation
replace: Should property accessors be replaced or not? #2011
Description
- Rollup Plugin Name:
@rollup/plugin-replace - Rollup Plugin Version: 6.0.3
- Rollup Version: 4.62.0
- Operating System (or Browser): macOS 15.7.7 (local), unknown (stackblitz)
- Node Version: v22.22.3
- Link to reproduction (
⚠️ read below): https://stackblitz.com/edit/rollup-repro-amxnsxzx?file=dist%2Fmain.js&view=editor
Expected Behavior
Given this test added in #1938 merged into 6.0.3:
| // Should be replaced - typeof window followed by dot | |
| if (typeof window.document !== 'undefined') { | |
| console.log('has document'); | |
| } |
| description: 'should not replace when followed by valid identifier characters', | |
| options: { | |
| values: { | |
| 'typeof window': '"undefined"' | |
| }, | |
| preventAssignment: true | |
| } |
Comment strongly suggests it should produce the following replacement:
// Should be replaced - typeof window followed by dot
if ("undefined".document !== 'undefined') {
console.log('has document');
}Actual Behavior
The snapshot stored by vitest does not actually replace this:
plugins/packages/replace/test/snapshots/form.js.md
Lines 43 to 46 in 639f456
| // Should be replaced - "undefined" followed by dot␊ | |
| if (typeof window.document !== 'undefined') {␊ | |
| console.log('has document');␊ | |
| }` |
And, there is an older fixture + snapshot which directly conflicts with this interpretation, under replace-nothing:
| console.log('as-it'); // eslint-disable-line | |
| console.log(typeof window.document); // eslint-disable-line |
| description: 'replaces nothing', | |
| options: { | |
| 'typeof window': `"object"` | |
| } |
Vitest Snapshot:
plugins/packages/replace/test/snapshots/form.js.md
Lines 76 to 81 in 639f456
| ## replace-nothing: replaces nothing | |
| > Snapshot 1 | |
| `console.log('as-it'); // eslint-disable-line␊ | |
| console.log(typeof window.document); // eslint-disable-line` |
Additional Information
The older PR which introduced the replace-nothing test was #903 which had the following comment:
Personally, I believe it's dangerous to replace
VERSIONin the case you highlighted by default:if (VERSION.startsWith('foo')) { // things }In the case above with the previous default to match VERSION when used with a
.or nested access, you would need to know all of your dependencies' code, not to mess up when replacing.
Originally posted by @o-alexandrov in #903 (comment)
What is the current intended behavior? I don't think these two tests can co-exist unless they're testing differently configured delimiters.
Activity
shellscape commentedon Jun 19, 2026
Thanks for the issue. I think you would be better served by replying to the PR and tagging the PR author.