feat!: convert source to native ES modules with dual ESM/CJS build#1322
Open
alexander-akait wants to merge 13 commits into
Open
feat!: convert source to native ES modules with dual ESM/CJS build#1322alexander-akait wants to merge 13 commits into
alexander-akait wants to merge 13 commits into
Conversation
Source files in `src/` are now native ES modules and the package
declares `"type": "module"`. A new Babel-driven dual build produces:
- `dist/esm/` (`.js` ESM, governed by the root package's
`"type": "module"`)
- `dist/cjs/` (`.js` CommonJS, with a `dist/cjs/package.json`
marker written by the build script so Node treats those files as
CommonJS regardless of the parent package type)
The `exports` field wires them up: `import` resolves to the ESM
build, `require` to the CJS build, and `default` to ESM for any
forward-looking tooling that matches neither condition.
The Babel config (`babel.config.cjs`) carries no custom plugins -
just `@babel/preset-env` toggled per `--env-name esm|cjs`. The
`build:cjs` script chains a tiny inline `node -e` to drop the
`{ "type": "commonjs" }` marker into `dist/cjs/package.json`.
`src/utils.js` now uses `await import("sass-embedded")` /
`await import("sass")` for resolving the sass implementation, so
both `getDefaultSassImplementation` and `getSassImplementation`
are async (this is breaking for tools that re-used the exported
`getSassImplementation`; they need to `await` it). The legacy
`src/cjs.js` shim is gone; tests now point webpack at
`dist/cjs/index.js` directly, which means tests depend on the
build (`pretest:base` runs `npm run build`).
The JSON schema is now `src/options.js` (a plain `export default`)
instead of `src/options.json`, to keep its definition co-located
with the rest of the (now ESM) source and avoid the upstream
eslint parser's lack of support for the `with { type: "json" }`
import-attribute syntax.
`require("sass-loader")` now returns the ES module namespace
(`{ __esModule: true, default: loader }`) rather than the loader
function. webpack 5's loader-runner auto-unwraps `module.default`,
so loader use is unchanged; callers using `require("sass-loader")`
directly need `.default`.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
The package now declares `"type": "module"`, so `.js` files are already treated as ESM by Node. The explicit `.mjs` / `.cjs` extensions on configs, tests, snapshots, and the changelog generator no longer carry information that the file extension doesn't already imply. - `babel.config.cjs` and `lint-staged.config.js` are now ESM modules (`export default`) under their `.js` names. - `eslint.config.mjs` is now `eslint.config.js` (content unchanged beyond the rename). - Every `test/*.test.mjs` becomes `test/*.test.js`, with matching `.snap` files renamed in lockstep so `node:test`'s snapshot resolver still finds them; `test/setup-snapshots.mjs` → `test/setup-snapshots.js` and the `test:base` script updated to match. - `.changeset/changelog-generator.mjs` → `.changeset/changelog-generator.js`, with `.changeset/config.json` pointing at the new filename. `test/helpers/testLoader.cjs` keeps its `.cjs` extension on purpose — webpack loads it directly via the CJS loader-runner path, so it needs to stay CommonJS regardless of the surrounding package type. https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
The tests now exercise the actual ESM source (`src/index.js`) instead
of the built CJS bundle. webpack 5's loader-runner picks ESM loaders
up via dynamic `import()`, which works fine under `node:test` (no
jest VM that needs `--experimental-vm-modules`).
Loading the schema is now `import schema from "./options.json" with
{ type: "json" }`. ESLint's effective `ecmaVersion: 2024` rejects the
`with` keyword, so `eslint.config.js` now pins the language to
`ecmaVersion: "latest"`. `src/options.js` is gone again - the import
attribute is enough.
`src/utils.js` keeps using dynamic `import()`, but with a small
`dynamicImport()` helper that falls back from `mod.default` to the
namespace itself. Real ESM imports of `sass-embedded` expose the
module via `.default`; CJS-shaped namespaces (the result of
`await import("/path/to/sass.node.js")` or anything Node treats as
already-an-ES-module via `__esModule: true`) expose the same methods
at the top level. The fallback works in both cases.
Two test helpers (`getImplementationByName`,
`getImplementationsAndAPI`) now use `await import("sass-embedded")`
so the test holds the *same* module instance the loader picks up.
The `require` and `import` paths in the `sass-embedded` package's
`exports` field resolve to separate CJS and ESM files; without the
realignment, `mock.method` on the CJS copy never fires in the ESM
loader. `getImplementationByName` is therefore async; callers were
updated to `await` it. `test/implementation-option.test.js` resolves
all implementations once at top level to avoid `await` inside a
non-async `for` body.
The `build:cjs` script now appends two lines to the compiled CJS
entry:
module.exports = exports.default;
module.exports.default = exports.default;
so `require("sass-loader")` once again returns the loader function
directly (with `.default` pointing back at it for transitional
consumers), matching the pre-refactor shape. A new test case in
`test/cjs.test.js` locks this in by loading the loader through both
`require()` and `import()` and asserting both surfaces are callable
functions with consistent `.default`/`.name`/`.length`.
Snapshot for `should throw error when unresolved package` was
updated: dynamic `import()` reports the missing package via
`ERR_MODULE_NOT_FOUND` (`Cannot find package 'unresolved' imported
from ...`) instead of the require-style `Cannot find module
'unresolved'`. All other error-message snapshots reference
`../src/index.js` now that webpack loads the loader from there.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
|
🦋 Changeset detectedLatest commit: 82b4fd1 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1322 +/- ##
==========================================
+ Coverage 96.64% 96.93% +0.29%
==========================================
Files 3 2 -1
Lines 834 848 +14
==========================================
+ Hits 806 822 +16
+ Misses 28 26 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When `options.implementation` is a string, the loader passes it straight
into `await import(...)`. ECMAScript's dynamic import only accepts
bare specifiers and `file:` URLs, so a raw filesystem path works on
POSIX (the Node loader is lenient) but fails on Windows where the path
has a drive letter and backslashes (`C:\\path\\to\\sass.js`).
A new `normalizeImportSpecifier` helper in `src/utils.js` runs the
specifier through `path.isAbsolute` + `url.pathToFileURL` when it
isn't already a `file:` URL. `pathToFileURL` is platform-aware, so on
Windows the result is a proper `file:///C:/...` URL.
Two new integration tests cover the surface:
- `'sass_file_url', 'modern' API` (added to
`test/implementation-option.test.js` via a new `sass_file_url` case
in `test/helpers/getImplementationByName.js`) feeds the loader a
`file:` URL directly to confirm pass-through.
- `should ship a CommonJS-transpiled bundle that require() returns`
(added to `test/cjs.test.js`) asserts the published artifact is
actually Babel-CJS — `dist/cjs/package.json` is
`{ "type": "commonjs" }`, the file starts with `"use strict";`,
it contains `exports.default = loader`, the post-build
`module.exports = exports.default;` line is present, and
`require()`-ing the file returns a function whose `.default`
property is itself.
The existing `sass_string` case already exercises the absolute-path
form on POSIX; on Windows it now reaches `pathToFileURL` instead of
being handed directly to `import()`.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
`test:base` no longer runs `npm run build` first. The webpack-driven
tests already point at `src/index.js` (the ESM source), so they
never needed `dist/`; `test/sourceMap-options.test.js` was the last
file that still referenced `dist/cjs/index.js` as a loader path, and
that now points at `src/index.js` too.
`test/cjs.test.js` previously read `dist/cjs/index.js` and
`dist/cjs/package.json` straight off disk, which made the CJS shape
assertions depend on an out-of-band `npm run build`. It now runs the
build itself, in-process:
- walks `src/`, hands every `.js` file to `transformFileAsync` with
`envName: "cjs"` (the same Babel config the published build uses),
copies non-JS files (`options.json`) verbatim,
- writes `package.json` with `{ "type": "commonjs" }`,
- appends the same `module.exports = exports.default;` /
`module.exports.default = exports.default;` lines `build:cjs`
writes after Babel.
The output lands in a fresh `os.tmpdir()` directory (cleaned up in
`after()`), then `require()`s it to assert the runtime shape: a
callable function whose `.default` points back at itself, matching
the pre-refactor `require("sass-loader")` contract.
This means a clean clone with no `dist/` can still `npm test` and
get full coverage of the CJS pipeline.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
…dule
`should try to load using valid order` and `should not swallow an error
when trying to load a sass implementation` had been skipped since the
jest -> node:test migration on main, with the reason
`blocked by test/node_modules/sass fixture (mock.module needs ESM
resolution)`.
The blocker was real but only partial:
- `test/node_modules/sass` is a deliberately invalid fixture (no
`package.json`) used by the webpack-`~sass/...` tests; it must stay.
- `mock.module("sass", ...)` issued from inside `test/` resolves the
bare specifier from the test file's location, hits the fixture, and
fails before the mock can register.
- However, `mock.module(<file: URL>, ...)` is keyed directly on the
URL. Any subsequent `await import(...)` that resolves to the same
URL — including the loader's `await import("sass-embedded")` /
`await import("sass")` issued from `src/utils.js` — gets the mock.
The fix is a small `resolveEsmEntry(specifier)` helper at the top of
the test file. It uses `node:module#findPackageJSON` to find the
package anchored at `src/utils.js` (which sidesteps the test fixture
entirely), walks the `exports` field preferring the `node` / `import` /
`default` conditions, and returns a `file:` URL of the package's ESM
entry. That URL is the same one Node's loader picks for the runtime
`await import(...)`, so a `mock.module(url, ...)` registration is
visible to the loader.
The two tests now do actual work:
- `should try to load using valid order` registers fake
sass-embedded and sass modules whose `default` exports declare
plausible `info` strings (`"sass-embedded\\t...\\t[Mocked]"` and
`"dart-sass\\t...\\t[Mocked]"`) plus a `__marker` field, then
re-imports `src/utils.js` (cache-busted via a query string) and
asserts `getSassImplementation({}, undefined)` returns the
sass-embedded mock — proving the preference order.
- `should not swallow an error when trying to load a sass
implementation` mocks sass-embedded's `default` export with a
`Proxy` whose `get` trap throws `Some error sass-embedded` for any
real property read (`then` and `symbol`-keyed protocol probes are
intentionally let through so the Promise machinery / interop hooks
don't trip the trap). The loader's `await import(...)` succeeds and
returns the proxy, but the next property access (`const { info } = ...`)
triggers the throw, and the test asserts that error surfaces
instead of being silently swallowed by a `sass` fallback.
Both tests `try/finally` so the mocks restore themselves even on
assertion failure.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
Previously the CJS build lived in `dist/cjs/` with a sibling
`package.json` marker (`{ "type": "commonjs" }`) so Node would treat
the `.js` files inside as CommonJS even though the root package is
`"type": "module"`. The `exports` field already steers consumers to
the right entry, so the nested directory only existed to satisfy
Node's per-file module-type resolution.
Switching the CJS build's output extension to `.cjs` removes that
need: `.cjs` is itself an unambiguous CommonJS extension regardless
of the surrounding package's type. Now the dist tree is flat:
dist/
index.js (ESM, governed by the root `"type": "module"`)
index.cjs (CJS, extension forces CommonJS)
utils.js (ESM)
utils.cjs (CJS)
options.json (shared by both)
To make this work we need every internal `import "./utils.js"` in
the CJS bundle to be rewritten to `require("./utils.cjs")` — Node's
CJS resolver doesn't try `.cjs` after `.js` fails. A small inline
Babel plugin (`rewriteRelativeJsToCjs`) walks `ImportDeclaration` /
`ExportNamedDeclaration` / `ExportAllDeclaration` before
`preset-env` rewrites them to `require()`, so the bundle ends up
with matching `.cjs` extensions on both filenames and require
paths. `.json` and bare specifiers are untouched.
`build:cjs` now passes `--out-file-extension .cjs` and emits into
`dist/` directly; the `node -e` post-build append now targets
`dist/index.cjs`. `build:esm` and `build:cjs` run sequentially
(`-s`) instead of in parallel so they don't race on the shared
`options.json` copy. `main`, `exports.import`, `exports.require`,
and `module` in `package.json` are updated to the flat paths.
`test/cjs.test.js`'s `buildCjsBundle` mirrors the new shape: it
writes each transpiled file as `<name>.cjs`, does not emit a
`package.json` marker, and asserts the bundle contains a literal
`require("./utils.cjs")` (proving the rewrite plugin ran).
Full suite still passes cold (`rm -rf dist && npm test`): 1157
pass / 0 fail / 0 skipped across 10 suites.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
The upstream `test: refactor` commits switched the test suite (and
the helper that supplies `implementation`) to `import * as sass from
"sass"` / `import * as sassEmbedded from "sass-embedded"`. ESM
namespace properties are non-configurable, so `node:test`'s
`mock.method` throws "Cannot redefine property: compileStringAsync"
the moment a suite tries to install a spy. Tests broke on every
implementation-option case.
To restore the spy mechanism without going back to the require-based
references, the helper now exposes mutable copies of each namespace
(via a `namedExportsOnly` shim that omits `default`, which `mock.module`
otherwise tries to render as `export const default = ...`). Both
`test/implementation-option.test.js` and the helper consume the same
plain object so `mock.method` mutations land on the live target.
`test/implementation-option.test.js` additionally calls
`mock.module(<file: URL>, { namedExports, defaultExport })` for the
sass / sass-embedded ESM entries — keyed by URL via
`findPackageJSON` anchored at `src/utils.js`, since the `sass`/`sass-test`
fixture rename now lets that resolution succeed. The same wrapper
object is passed as both `namedExports` and `defaultExport`, so the
loader's `await import(...) → mod.default ?? mod` ends up with the
exact object the test mutates. Test mutations to `.info`,
`compileStringAsync`, `initAsyncCompiler`, etc. propagate to the
loader through one live reference. The two previously
mock-module-by-URL tests (`should try to load using valid order`,
`should not swallow ...`) now mutate that same wrapper in place via
`Object.defineProperty` / property assignments and restore in
`finally`, because the suite-level `mock.module` already owns the URL.
The CJS build needed `.default ?? mod` for both the bare-specifier
and the string-path branches, since `await import(absolutePath)` of a
CJS file surfaces the implementation under `.default`. And
`getSassImplementation`'s `try / catch` was narrowed to only fall
back from `sass-embedded` to `sass` when the failure is
`ERR_MODULE_NOT_FOUND` / `MODULE_NOT_FOUND` — any other failure
surfaces, matching the "should not swallow an error" contract.
The flat-dist `babel.config.js` is preserved: `rewriteRelativeJsToCjs`
runs before `@babel/plugin-transform-modules-commonjs`
(`ignoreDynamicImport: true`, kept from upstream), so the CJS output
gets `.cjs` extensions on filenames AND on the resulting `require()`
paths, while the loader's `await import(...)` survives intact in the
transpiled output.
Cold run (`rm -rf dist && npm test`): 1158 pass / 0 fail / 0 skipped
across 10 suites. Lint clean.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
Per your direction: don't touch babel, and don't reach for `.default`
on `sass` / `sass-embedded` (the namespace is the supported surface;
`import sass from "sass"` was deprecated upstream).
`babel.config.js`, `package.json`, `test/cjs.test.js`, and
`src/utils.js` are restored to the upstream state from `67f8f43` —
back to the separate `dist/esm/` + `dist/cjs/` layout with
`@babel/plugin-transform-modules-commonjs` (`ignoreDynamicImport:
true`) and the `dist/cjs/package.json` type marker. No more inline
rewrite plugin, no more `.cjs` extension, no more flat dist.
`src/utils.js` no longer reaches `.default` anywhere. The path-form
implementation handler stays as a plain
`await import(normalizeImportSpecifier(...))`. To make
`'sass_string'` and `'sass_file_url'` work without `.default`,
`test/helpers/getImplementationByName.js` now resolves the *ESM*
entry (walking the package's `exports` `node`/`import`/`default`
conditions) instead of returning `require.resolve("sass")` (which
points at the CJS `sass.node.js` and exposes only `default` when
imported dynamically). The loader's `await import(esmPath)` gets a
real namespace with `info`, `compileStringAsync`, etc. — no
unwrapping needed.
`getSassImplementation`'s `try / catch` is narrowed to
`ERR_MODULE_NOT_FOUND` / `MODULE_NOT_FOUND` only. Any other failure
during `await import("sass-embedded")` surfaces, matching the
"should not swallow an error" contract. This is the one loader
behavior change beyond the revert; without it the test would still
fall back silently to `sass`.
Full suite: 1158 pass / 0 fail / 0 skipped across 10 suites. Lint
clean.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
Node 26 deprecated `mock.module()`'s `namedExports` / `defaultExport`
options and introduced a unified `exports` option. The new option
**snapshots** values at synthesis time, so the existing live-mutation
pattern (suite-level `mock.module(URL, { namedExports: wrapper })` →
later `mock.method(wrapper, ...)`) silently stopped propagating to
the loader's `await import(...)` view. CI's Node 26 leg failed on
nine implementation-option tests; the rest were cancelled by the
failure.
`mockModuleOptions(target)` picks per Node major:
- Node 22 / 24: keep the old `{ namedExports, defaultExport }` shape
(mock.module reads keys live there).
- Node 26+: build an `exports` object whose every key — including
`default` — is a getter that reads through to `target[key]`. Tests
mutate `target`, the getter reads the current value, the mocked
module returns it.
The two shapes can't be mixed: passing both on Node 26 errors out.
Picking by `process.versions.node` keeps both lanes happy.
`test/implementation-option.test.js` passes:
- Node 22.11 / 22.22: 1158 pass / 0 fail / 0 skipped
- Node 26.1.0: 1158 pass / 0 fail / 0 skipped
Lint clean.
https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
The previous fix split per Node major: `namedExports` for < 26, getter-backed `exports` for >= 26. Node 24's CI leg then failed because Node 24 *also* snapshots `namedExports` (the option is deprecated there too — only Node 22 still reads it live). A probe on Node 24.15.0: | Option | Behavior on Node 22 | 24 | 26 | | -------------------------- | ------------------- | -- | -- | | `namedExports` (live wrap) | LIVE | snapshot | (deprecated/snapshot) | | `exports` (plain spread) | option ignored | snapshot | snapshot | | `exports` (getter wrap) | option ignored | LIVE | LIVE | So the cutoff is *Node 22 only* for `namedExports`; Node 24+ has to use the getter-backed `exports` form. Lower the version branch to `NODE_MAJOR < 24` accordingly. Verified on Node 22.22.2, 24.15.0, 26.1.0: 1158 pass / 0 fail / 0 skipped each. https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Source files in
src/are now native ES modules and the packagedeclares
"type": "module". A new Babel-driven dual build produces:dist/esm/(.jsESM, governed by the root package's"type": "module")dist/cjs/(.jsCommonJS, with adist/cjs/package.jsonmarker written by the build script so Node treats those files as
CommonJS regardless of the parent package type)
The
exportsfield wires them up:importresolves to the ESMbuild,
requireto the CJS build, anddefaultto ESM for anyforward-looking tooling that matches neither condition.
The Babel config (
babel.config.cjs) carries no custom plugins -just
@babel/preset-envtoggled per--env-name esm|cjs. Thebuild:cjsscript chains a tiny inlinenode -eto drop the{ "type": "commonjs" }marker intodist/cjs/package.json.src/utils.jsnow usesawait import("sass-embedded")/await import("sass")for resolving the sass implementation, soboth
getDefaultSassImplementationandgetSassImplementationare async (this is breaking for tools that re-used the exported
getSassImplementation; they need toawaitit). The legacysrc/cjs.jsshim is gone; tests now point webpack atdist/cjs/index.jsdirectly, which means tests depend on thebuild (
pretest:baserunsnpm run build).The JSON schema is now
src/options.js(a plainexport default)instead of
src/options.json, to keep its definition co-locatedwith the rest of the (now ESM) source and avoid the upstream
eslint parser's lack of support for the
with { type: "json" }import-attribute syntax.
require("sass-loader")now returns the ES module namespace(
{ __esModule: true, default: loader }) rather than the loaderfunction. webpack 5's loader-runner auto-unwraps
module.default,so loader use is unchanged; callers using
require("sass-loader")directly need
.default.https://claude.ai/code/session_01XWs5VsKDPMo38kskxHoiRM