feat: support CommonJS reexports via Object.defineProperty descriptors#21129
Conversation
Detect `Object.defineProperty(exports, "name", { value: require("...") })`
as a structured reexport and implement the previously-stubbed
`Object.defineProperty` branch in CommonJsExportRequireDependency, so these
reexports participate in exports analysis and tree-shaking like the
`exports.name = require(...)` form.
https://claude.ai/code/session_01Tprbu7T8nKkms2TTmn6jpM
Extend the `Object.defineProperty(exports, name, ...)` reexport detection to
the lazy `{ get: () => require("...") }` / `{ get() { return require("...") } }`
accessor form used by barrel files such as webpack's own `lib/index.js`,
alongside the eager `{ value: require("...") }` form. Lazy getters preserve
their deferred semantics in the generated code and unused ones are dropped
without running the underlying module.
Adds tree-shaking cases (arrow/function/method getters, laziness, deep ids)
and an ESM `library.type: "module"` case that statically named-imports both
value and getter reexports from a CommonJS barrel.
https://claude.ai/code/session_01Tprbu7T8nKkms2TTmn6jpM
A `{ get: () => require(...), set }` descriptor was treated as a getter
reexport, which rebuilt the descriptor with only the getter and silently
dropped the setter (assignment then throws under strict/ESM output). Bail out
of the reexport rewrite when a `set` accessor is present so the descriptor is
kept verbatim by the generic handler.
https://claude.ai/code/session_01Tprbu7T8nKkms2TTmn6jpM
🦋 Changeset detectedLatest commit: 10e0689 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR is packaged and the instant preview is available (871e391). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@871e391
yarn add -D webpack@https://pkg.pr.new/webpack@871e391
pnpm add -D webpack@https://pkg.pr.new/webpack@871e391 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21129 +/- ##
==========================================
- Coverage 92.33% 92.33% -0.01%
==========================================
Files 581 581
Lines 62942 62991 +49
Branches 17420 17444 +24
==========================================
+ Hits 58117 58162 +45
- Misses 4825 4829 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by ×3.5
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | benchmark "asset-modules-bytes", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
858.9 KB | 245.3 KB | ×3.5 |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/search-todo-webpack-6-y0F9O (10e0689) with main (d39efba)
Summary
The
Object.definePropertybranch ofCommonJsExportRequireDependencywas an unimplementedthrow new Error("TODO"), soObject.defineProperty(exports, name, { value: require("...") })re-exports were never treated as structured re-exports (no exports analysis, no tree-shaking). This PR detects and implements them, including the lazy{ get: () => require("...") }/{ get() { return require("...") } }barrel form used by webpack's ownlib/index.js. Lazy getters keep their deferred semantics in the generated code, and a{ get, set }descriptor keeps its setter (it falls back to the generic handler rather than silently dropping it).What kind of change does this PR introduce?
feat (plus a related fix for the get/set descriptor case).
Did you add tests for your changes?
Yes —
test/cases/cjs-tree-shaking/reexports-define-property/(value/getter/get+set forms across theexports/module.exports/thisbases, nested ids, laziness, unused) and two library cases:test/configCases/library/reexport-define-property/(CommonJS output) andtest/configCases/library/reexport-define-property-module/(ESMlibrary.type: "module"output, statically named-importing value and getter reexports from a CommonJS barrel).Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a — internal exports-analysis optimization, no public API or config change.
Use of AI
Yes — implemented with Claude Code (Anthropic). All changes were reviewed, run, and tested locally before submitting.
Generated by Claude Code