replace: Should property accessors be replaced or not?
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 35/100
- issue の種類
- バグ
- 明瞭さ
- 説明が足りない
- 活発さ
- 静か
- 技術スタック
- javascript
- 領域
- tooling
調査の方向性
Compare packages/replace/test/fixtures/form/dollar-sign-boundary and replace-nothing with their _config.js files and the Vitest snapshots under packages/replace/test/snapshots/form.js.md, using the linked reproduction as context. First establish which property-access and delimiter behavior is intended; done means the conflicting fixtures and snapshots consistently reflect the agreed behavior.
索引モデルが issue の本文から書いたものです。
説明
- 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:
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/replace/test/fixtures/form/dollar-sign-boundary/input.js#L17-L20
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/replace/test/fixtures/form/dollar-sign-boundary/_config.js#L2-L8
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:
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/replace/test/snapshots/form.js.md?plain=1#L43-L46
And, there is an older fixture + snapshot which directly conflicts with this interpretation, under replace-nothing:
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/replace/test/fixtures/form/replace-nothing/input.js#L1-L2
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/replace/test/fixtures/form/replace-nothing/_config.js#L2-L5
Vitest Snapshot:
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/replace/test/snapshots/form.js.md?plain=1#L76-L81
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 https://github.com/rollup/plugins/issues/903#issuecomment-885657501
What is the current intended behavior? I don't think these two tests can co-exist unless they're testing differently configured delimiters.
- 主要言語
- JavaScript
- スター
- 3.8k
- フォーク
- 635
- PR マージ指標
- 30日以内にマージされた PR はありません
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
rollup/plugins のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
-
難易度 4/5 3〜5日 初心者へのやさしさ 52/100
-
難易度 3/5 1〜2日 初心者へのやさしさ 48/100
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
-
Support js-yaml v5 オープン
難易度 4/5 3〜5日 初心者へのやさしさ 45/100
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
HarperFast/skills#96 ·
-
[Block] Latest Posts [Type] Bug
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
Automattic/studio#4908 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 86/100
sugarlabs/musicblocks#8847 ·