Skip to content

Update uglify-js dep#3016

Merged
sokra merged 1 commit into
webpack:masterfrom
SpaceK33z:upgrade-uglifyjs
Sep 19, 2016
Merged

Update uglify-js dep#3016
sokra merged 1 commit into
webpack:masterfrom
SpaceK33z:upgrade-uglifyjs

Conversation

@SpaceK33z

Copy link
Copy Markdown
Member

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

See #3011, code mangling doesn't work.

What is the new behavior?

Manging works with new uglify-js.

Does this PR introduce a breaking change?

  • Yes
  • No

@SpaceK33z

Copy link
Copy Markdown
Member Author

Hm, AppVeyor failed with an unrelated error it seems?

@sokra sokra merged commit 159ed35 into webpack:master Sep 19, 2016
@sokra

sokra commented Sep 19, 2016

Copy link
Copy Markdown
Member

Thanks

alexander-akait added a commit that referenced this pull request May 19, 2026
Adds an `underscore-like.mjs` fixture — a callable library that exports
itself both as `default` and `"module.exports"`, plus an `_.partial` that
relies on the library function being its own placeholder identity. This
mirrors the exact shape of the upstream issues the PR description links
to (underscore/issues/3016 — `_.partial.placeholder === _` broke when
bundlers turned `_` into an ESM namespace; esbuild/issues/4459 —
`const _ = require("underscore")` returning a namespace instead of the
callable library).

Four new cases pin the behavior down:

  * `const _ = require(lib)` yields the callable library function.
  * `_.partial.placeholder === _` (the underscore #3016 identity check).
  * `_.partial(fn, _, x, _)` correctly substitutes the placeholder.
  * Destructured pull behaves the same as a default import would.

Each case is cross-checked against `Module._load` so any future drift
against Node's `require(esm)` would fail here.

No new lib changes — this is the smallest reproduction of the original
issue's use case wired into the regular CI run.
alexander-akait added a commit that referenced this pull request May 20, 2026
…20981)

* feat: support `require(esm)` "module.exports" named-export interop

When CommonJS `require()` resolves to an ES module that exports a binding
with the literal string name `"module.exports"` (e.g.
`export { value as "module.exports" }`), webpack now returns that export's
value instead of the module's namespace object — matching Node.js v23+
`require(esm)` semantics and easing migration of dual ESM/CJS libraries
that depend on `module.exports = …`.

The unwrapping is wired through the three CommonJS require dependency
templates so it applies to plain `require()`, property access
(`require().foo`), call (`require()(…)`), and destructuring. Tree-shaking
sees `"module.exports"` as referenced when applicable.

Closes #20896

* test: compare `require(esm)` unwrapping against real Node behavior

The `require(esm)` "module.exports" interop test now cross-checks
webpack's bundled output against Node.js's native result by shelling
out to a child `node` process — Jest's runtime intercepts every
`require()` in-process (even via `Module.createRequire`), so the
comparison has to run outside it. The test also keeps a literal
`require(/* webpackIgnore: true */ … )` in the entry to assert webpack
preserves the comment in the emitted bundle.

A `test.filter.js` skips the case on Node versions older than 22.12.0,
where `require(esm)` is not unflagged.

`.js` fixtures are renamed to `.mjs` so the child `node` process parses
them as ES modules.

* test: use `webpackIgnore` for in-process native `require(esm)` comparison

Replaces the child-process approach with a `webpackIgnore: true` comment
on the native side. Webpack leaves the call literal, and at runtime a
new `test.config.js` short-circuits the absolute paths via
`testConfig.modules` to values pre-loaded through Node's `Module._load`
(which, unlike `Module.createRequire`, Jest does not intercept).

The webpack-bundled `require("./*.mjs")` and the
`require(/* webpackIgnore: true */ pathVar)` calls thus go through two
independent paths and their results are compared, with no child
processes.

* test: cover `"module.exports"` precedence and re-export against Node

Adds two fixtures and corresponding cases that pin down behaviors the
Node.js docs guarantee but the previous tests did not exercise:

  * `"module.exports"` wins over a sibling `default` (and any other named
    export). The unwrapped value is just the binding — `default` /
    `named` are not visible on it.
  * `export { x as "module.exports" } from "./other"` still unwraps when
    required from CJS.

Both cases are cross-checked against `Module._load`'d native values.

* feat: extend `require(esm)` "module.exports" unwrap to CJS re-exports

`CommonJsExportRequireDependency` (the dependency behind
`module.exports = require(…)`, `module.exports.foo = require(…)`,
`exports.bar = require(…).baz`) now applies the same Node.js `require(esm)`
unwrap as plain `require()` / `require().foo`. When the imported module
is an ES module with a `"module.exports"` named export:

* `getReferencedExports` reports only `"module.exports"` as referenced;
  further property access in `ids` lands on the unwrapped value which
  webpack does not model.
* `getExports` prepends `"module.exports"` to the export chain when a
  single name is being re-exported, and falls back to `exports: true`
  (canMangle: false) for full re-exports — the unwrapped value's own
  properties cannot be enumerated statically.
* The template emits `__webpack_require__(id)["module.exports"]<ids>`
  in place of `__webpack_require__(id)<ids>`.

Three new fixtures cover the wrapper-CJS shapes against Node's real
`Module._load` result.

* chore: bump `require(esm)` "module.exports" changeset to patch

This change brings webpack into line with Node.js's documented
`require(esm)` semantics, so it's a fix for divergent behavior rather
than a new feature — patch is the right semver bucket.

* fix: decouple `require(esm)` unwrap eligibility from used-name lookup

`getRequireEsmModuleExportsAccess` previously bailed out when
`getUsedName(["module.exports"], runtime)` returned `false`, which created
a chicken-and-egg in `getReferencedExports`: the helper would refuse to
mark `"module.exports"` as the referenced export until it was already
marked used, so `usedExports: true` builds fell through to referencing
the user-side property (e.g. `named`) and emitted
`__webpack_require__(id).named` — disagreeing with Node's `require(esm)`,
which would have unwrapped first and then accessed `.named` on the
(string) value, yielding `undefined`.

Split into `isRequireEsmModuleExportsModule` (usage-independent, used by
the three `getReferencedExports` paths and `getExports`) and the existing
`getRequireEsmModuleExportsAccess` (which still runs the used-name
lookup, but only at template-apply time, when usage is final).

A new `distinct.mjs` fixture exports `"module.exports"` and `named` to
*different* values; the config now enables `optimization.usedExports`
so this regression is locked down on every run.

Spotted by the Copilot PR review.

* test: cover the Underscore-shaped library pattern from #20896

Adds an `underscore-like.mjs` fixture — a callable library that exports
itself both as `default` and `"module.exports"`, plus an `_.partial` that
relies on the library function being its own placeholder identity. This
mirrors the exact shape of the upstream issues the PR description links
to (underscore/issues/3016 — `_.partial.placeholder === _` broke when
bundlers turned `_` into an ESM namespace; esbuild/issues/4459 —
`const _ = require("underscore")` returning a namespace instead of the
callable library).

Four new cases pin the behavior down:

  * `const _ = require(lib)` yields the callable library function.
  * `_.partial.placeholder === _` (the underscore #3016 identity check).
  * `_.partial(fn, _, x, _)` correctly substitutes the placeholder.
  * Destructured pull behaves the same as a default import would.

Each case is cross-checked against `Module._load` so any future drift
against Node's `require(esm)` would fail here.

No new lib changes — this is the smallest reproduction of the original
issue's use case wired into the regular CI run.
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