Skip to content

fix(code-splitting): stop negated sideEffects globs retaining modules - #10660

Merged
IWANABETHATGUY merged 4 commits into
rolldown:mainfrom
JoviDeCroock:fix/negated-side-effects-globs
Aug 11, 2026
Merged

fix(code-splitting): stop negated sideEffects globs retaining modules#10660
IWANABETHATGUY merged 4 commits into
rolldown:mainfrom
JoviDeCroock:fix/negated-side-effects-globs

Conversation

@JoviDeCroock

Copy link
Copy Markdown
Contributor

Package sideEffects patterns form a positive allowlist, but fast_glob interprets a leading exclamation mark as a negation. Passing those patterns through made each one match nearly every unrelated module and prevented affected packages from tree-shaking.

Ignore leading-bang string and array patterns before matching, and cover the behavior with a generic unit and integration regression.

Note

This reproduction was created by a human, implemented by GPT5.6-sol and reviewed by a human

@JoviDeCroock JoviDeCroock changed the title fix: stop negated sideEffects globs retaining modules fix(code-splitting): stop negated sideEffects globs retaining modules Aug 10, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing JoviDeCroock:fix/negated-side-effects-globs (f5820c3) with main (4f0a589)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@IWANABETHATGUY IWANABETHATGUY self-assigned this Aug 11, 2026
Comment thread crates/rolldown_common/src/types/package_json.rs Outdated
@JoviDeCroock
JoviDeCroock force-pushed the fix/negated-side-effects-globs branch from 3780588 to e4df07b Compare August 11, 2026 04:13
@IWANABETHATGUY
IWANABETHATGUY enabled auto-merge (squash) August 11, 2026 04:25
JoviDeCroock and others added 4 commits August 11, 2026 12:25
Package sideEffects patterns form a positive allowlist, but fast_glob interprets a leading exclamation mark as a negation. Passing those patterns through made each one match nearly every unrelated module and prevented affected packages from tree-shaking.

Ignore leading-bang string and array patterns before matching, and cover the behavior with a generic unit and integration regression.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`!foo.js` is a legal POSIX path, and Rollup with @rollup/plugin-node-resolve
matches such a `sideEffects` entry literally. Escape the `!` before handing the
normalized pattern to fast_glob so it stays a literal match, rather than
filtering leading-bang patterns out entirely.

Co-Authored-By: IWANABETHATGUY <iwanabethatguy@qq.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@IWANABETHATGUY
IWANABETHATGUY force-pushed the fix/negated-side-effects-globs branch from e4df07b to f5820c3 Compare August 11, 2026 04:25
@netlify

netlify Bot commented Aug 11, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit f5820c3
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a7aa437a8779e00083120af

@IWANABETHATGUY
IWANABETHATGUY merged commit 464e820 into rolldown:main Aug 11, 2026
35 checks passed
@rolldown-guard rolldown-guard Bot mentioned this pull request Aug 12, 2026
shulaoda added a commit that referenced this pull request Aug 12, 2026
## [1.2.4] - 2026-08-12

### 🚀 Features

- code-splitting: widen namespace extraction for already-loaded merges (#10594) by @IWANABETHATGUY

### 🐛 Bug Fixes

- binding: illegal instruction crash on Android arm64 (#10638) by @shulaoda
- code-splitting: stop negated sideEffects globs retaining modules (#10660) by @JoviDeCroock
- devtools: resolve log paths against the build cwd (#10622) by @hyfdev

### 📚 Documentation

- plugin-api: document sourcemap differences from Rollup (#10636) by @IWANABETHATGUY

### 🧪 Testing

- browser: smoke test the packed `@rolldown/browser` in a real browser page (#10634) by @hyfdev
- node-wasi: run the full node test suite against the WASI binding (#10616) by @hyfdev
- webcontainer: smoke test the packed WASI artifacts inside a WebContainer (#10626) by @hyfdev

### ⚙️ Miscellaneous Tasks

- deps: update napi (#10649) by @renovate[bot]
- deps: update esbuild for tests to 0.28.2 (#10662) by @rolldown-guard[bot]
- deps: update test262 submodule for tests (#10663) by @rolldown-guard[bot]
- deps: update npm packages (#10653) by @renovate[bot]
- deps: update github actions (#10651) by @renovate[bot]
- deps: update crate-ci/typos action to v1.49.0 (#10642) by @renovate[bot]
- deps: update dependency vite-plus to v0.2.9 (#10627) by @renovate[bot]
- deps: update rust crates (#10652) by @renovate[bot]
- deps: upgrade oxc to 0.144.0 (#10657) by @camc314
- ignore napi-rs temp files (#10658) by @camc314
- deps: tighten `@napi-rs/wasm-runtime` range to `~1.2.2` (#10615) by @hyfdev
- wasi: point the wasm skips at the right place (#10644) by @hyfdev

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
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.

2 participants