Migrate all rules to ESM #7211
Closed
Migrate all rules to ESM #7211
Conversation
This file contains 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
I'll quickly explain the methodology for this PR:
1. remove each `lib/rules/*` for each deprecated test
2. remove each (now invalid) path from `lib/rules/index.js`
3. fix all broken tests, which fall into one of several categories:
- tests that use a deprecated rule as an arbitrary rule; to resolve this, I just picked a different rule (usually `color-named` for `color-hex-case` and `number-max-precision` for `indentation`)
- one of these is about a custom plugin, so I've rewritten the plugin
- tests that are *about* deprecated rules; I added a `.skip` (have left comments in the review)
- fs tests that use deprecated rules; I removed these from the config, then updated the snapshot.
- the zen garden one doesn't work "out of the box" anymore, i.e. the snapshot has nontrivially changed because of indentation/spacing rules
- `indentation`-specific behaviour (it being last); I deleted these!
4. remove all documentation related to removed rules (mostly `indentation`)
5. remove all unused utilities (via code coverage, not manual inspection)
Ref: #6979 (comment) Re-running the script indicates no newly-unused files: ```sh $ node scripts/find-unused-modules.mjs ```
Node.js 16.13.0 is the first version of Node.js 16.x LTS series. See https://nodejs.org/en/blog/release/v16.13.0
) `mergeTestDescriptions()` is used by deprecated rules. So I've created this commit from the `v16` branch, instead of the `main` branch. This change enables the `esModuleInterop` flag to prevent the following TypeScript error: ``` lib/testUtils/mergeTestDescriptions.mjs:1:8 - error TS1259: Module '"/Users/masafumi.koba/git/stylelint/stylelint/node_modules/deepmerge/index"' can only be default-imported using the 'esModuleInterop' flag 1 import merge from 'deepmerge'; ~~~~~ node_modules/deepmerge/index.d.ts:20:1 20 export = deepmerge; ~~~~~~~~~~~~~~~~~~~ This module is declared with 'export =', and can only be used with a default import when using the 'esModuleInterop' flag. ```
This change rewrites some tiny utility modules in the `lib/utils/` directory to ESM.
In addition to rewriting some utility modules to ESM, this change also fine-tunes Rollup to prevent inlined modules. See the `output.preserveModules` option for RollUp: https://rollupjs.org/configuration-options/#output-preservemodules
This change rewrites small utility modules (less dependent on 3rd-party packages) to ESM.
This change configures the following Rollup options to remove redundant helpers from CJS output files. - [`output.interop`](https://rollupjs.org/configuration-options/#output-interop) - [`output.esModule`](https://rollupjs.org/configuration-options/#output-esmodule) Additionally, this turns on the following option to remove unneeded Symbol helpers. - [`output.generatedCode.symbols`](https://rollupjs.org/configuration-options/#output-generatedcode-symbols)
Node.js 16 is 100% compatible with ES2021. See https://node.green/#ES2021 In addition, this change uses `String.prototype.replaceAll()` as possible for readability. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll
This change rewrites almost `lib/utils/*.js` files to ESM, except for files depending on `rules/index.js` and `formatters/index.js`. Also, this change disables the `treeshake` option of Rollup since optimization of output files is unneeded. See https://rollupjs.org/configuration-options/#treeshake
This change rewrites relatively simpler `lib/*.js` files to ESM.
`formatters/index` is internally uses `lazy-import`. This is a bit hard to resolve, so I will migrate it later.
…ems to `stdout`/`code` and `stderr`/`output` (#7119) Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
…ent outputs (#7131) This commit changes the following: 1. To avoid waiting for stdin without any input. For example, just running the `stylelint` command immediately exits, instead of waiting. I believe this behavior is more user-friendly. 2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty string input (`''`) from stdin. See the example below: ```sh # code is undefined bin/stylelint.mjs # code is '' echo -n '' | bin/stylelint.mjs ``` Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Note that this change still keeps using `import-lazy` to minimize this changeset.
) - Remove redundant `globals`. - Make `testRule*` globals readonly. - Turn on `n/prefer-global/process` to disallow global `process` for consistency. See https://github.com/eslint-community/eslint-plugin-n/blob/master/docs/rules/prefer-global/process.md - Bump eslint-config-stylelint to 20.0.0 Global variables are often harmful. Unlike global variables defined in standard specs (ECMAScript or WHATWG etc.), `process` is unique to Node.js. This may be a problem when running Stylelint on other platforms.
Bumps [postcss-safe-parser](https://github.com/postcss/postcss-safe-parser) from 6.0.0 to 7.0.0. - [Changelog](https://github.com/postcss/postcss-safe-parser/blob/main/CHANGELOG.md) - [Commits](postcss/postcss-safe-parser@6.0.0...7.0.0) --- updated-dependencies: - dependency-name: postcss-safe-parser dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [file-entry-cache](https://github.com/jaredwray/file-entry-cache) from 7.0.0 to 7.0.1. - [Release notes](https://github.com/jaredwray/file-entry-cache/releases) - [Commits](jaredwray/file-entry-cache@v7.0.0...v7.0.1) --- updated-dependencies: - dependency-name: file-entry-cache dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the eslint group with 1 update: [eslint](https://github.com/eslint/eslint). - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md) - [Commits](eslint/eslint@v8.50.0...v8.51.0) --- updated-dependencies: - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-minor dependency-group: eslint ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the jest group with 1 update: [jest-preset-stylelint](https://github.com/stylelint/jest-preset-stylelint). - [Release notes](https://github.com/stylelint/jest-preset-stylelint/releases) - [Changelog](https://github.com/stylelint/jest-preset-stylelint/blob/main/CHANGELOG.md) - [Commits](stylelint/jest-preset-stylelint@6.2.0...6.3.1) --- updated-dependencies: - dependency-name: jest-preset-stylelint dependency-type: direct:development update-type: version-update:semver-minor dependency-group: jest ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
mattxwang
approved these changes
Oct 16, 2023
There was a problem hiding this comment.
Sorry for the delay! I've sat down and taken as good of a look at 131dc13 that I think I can, by:
- looking at
rewrite-rules-to-esm.mjs, which seems correct - manually taking a look at a few of the edited files to look for errors
With test coverage, I feel relatively confident that this is good to merge. However, it'd be great to have another set of eyes to approve this as well (perhaps @Mouvedia, who has also been pretty active recently)?
Appreciate you taking on this huge task @ybiquitous!
Mouvedia
reviewed
Oct 17, 2023
There was a problem hiding this comment.
@mattxwang I tried.
@ybiquitous it seems you used a tool or script so I couldn't find much.
lib/rules/font-family-no-missing-generic-family-keyword/index.mjs
Outdated
Show resolved
Hide resolved
|
Since this PR has so many conflicts, I'll split this into several PRs. |
|
I'll merge extracted PRs when CI passes to avoid pain due to so many conflicts. EDIT: See also #7002 |
This was referenced Oct 19, 2023
This was referenced Oct 19, 2023
Merged
|
This PR is no longer needed since PR #7262 has been merged. |
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.
Ref #5291
I created and performed a temporary script to rewrite all rules, but several commits were needed for preparation. Please look at each commit to confirm my workflow, especially 131dc13.
Note: The test coverage temporarily degrades, but it's due to
jest-preset-stylelint. See #7182 (comment).The next step after this PR will be to remove
import-lazyinlib/rules/index.mjslike #7184:stylelint/lib/rules/index.mjs
Line 5 in 8e66566