feat!: rewrite as ECMAScript modules - #308
Merged
Merged
Conversation
`src/` is ECMAScript modules, the package declares `"type": "module"` and an
`exports` field, and babel emits an ESM build beside a CommonJS one, so
`import LintPlugin from "lint-webpack-plugin"` and `require("lint-webpack-plugin")`
both keep working.
Four things the rewrite forced, each the smallest fix that holds:
`"type": "module"` decides how every `.js` in the repository is parsed, so the
CommonJS test fixtures and mocks stopped being CommonJS and webpack quietly
built no dependency for their `require`. Each of those directories now carries
a `package.json` naming itself `commonjs`.
The stylelint worker entry stays CommonJS as `stylelint-worker.cjs`. jest
declines to `require` an ES module below node 24.9, which is what jest-worker
does when it loads a worker in band, and a worker entry has no reason to be
anything else.
`eslintPath` and `stylelintPath` may name a directory or a CommonJS entry, which
ESM resolution does not find. `importFrom` resolves those through CommonJS
first, and imports a bare package name as it is so that it can still be mocked.
The schemas are read with `createRequire` rather than an import attribute, which
`eslint-plugin-import` cannot yet parse.
BREAKING CHANGE: the package is now `"type": "module"` with an `exports` field.
Deep imports into the package are no longer reachable; the plugin and
`./package.json` are what it exports.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
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.
Summary
src/becomes ECMAScript modules, the package declares"type": "module"and anexportsfield, and babel emits an ESM build beside a CommonJS one — soimport LintPlugin from "lint-webpack-plugin"andrequire("lint-webpack-plugin")both keep working. The layout followsless-loader:dist/esmanddist/cjs, each stamped with its ownpackage.json,mainpointing at the CommonJS entry for old resolvers.Four things the rewrite forced, each worth a reviewer's attention because none of them is obvious from the diff:
"type": "module"decides how every.jsin the repository is parsed, not justsrc/. The CommonJS test fixtures and mocks stopped being CommonJS, so webpack built no dependency for theirrequire(...)and the plugin silently linted only entry files. Those directories now carry apackage.jsonnaming themselvescommonjs.stylelint-worker.cjs. jest declines torequirean ES module below node 24.9 — which is what jest-worker does when it loads a worker in band — and a worker entry has no reason to be anything else. Node itself loads the ESM version fine; this is jest's limit, verified by driving jest-worker against it outside jest.eslintPathandstylelintPathmay name a directory or a CommonJS entry, neither of which ESM resolution finds.importFromresolves those through CommonJS first, and imports a bare package name as it is so a test can still mock it.createRequirerather than animport ... with { type: "json" }attribute, whicheslint-plugin-importcannot yet parse.What kind of change does this PR introduce?
feat (breaking).
Did you add tests for your changes?
No new tests — the existing suite is the check, and it drove the whole migration. 124 passing, 2 skipped (the ESLint 10 eslintrc suites, as on main). Three tests changed shape rather than intent:
errorandeslint-lintmocked theeslintmodule throughjest.mock, which ESM does not support, and now use the plugin's owneslintPathwith a mock — the same pattern the stylelint tests already use.utilskeeps module mocking throughjest.unstable_mockModule.Beyond the suite I verified both published entry points load (
importfromdist/esm,requirefromdist/cjs) and that the built worker keeps its.cjsextension in both builds.Does this PR introduce a breaking change?
Yes. The package is
"type": "module"with anexportsfield, so deep imports into it are no longer reachable — the plugin and./package.jsonare what it exports. Consumers importing or requiring the package itself are unaffected.If relevant, what needs to be documented once your changes are merged or what have you already documented?
The README's setup example now leads with
importand notes that a CommonJS configuration works too. A changeset is included marking this a major.Use of AI
Written with Claude Code, driven interactively. I asked for the rewrite; Claude did the conversion and worked the test suite back to green, reporting the four constraints above as it hit them. I reviewed the result.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
Generated by Claude Code