Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update configuration for ESM #133

Merged
merged 1 commit into from Sep 14, 2021
Merged

Update configuration for ESM #133

merged 1 commit into from Sep 14, 2021

Conversation

ybiquitous
Copy link
Member

Which issue, if any, is this issue related to?

Related to stylelint/stylelint#5291

Is there anything in the PR that needs further explanation?

We are moving to ESM on the stylelint/stylelint repo: stylelint/stylelint#5291
This update will be required for the migration.

I think some changes may be controversial. Please give me feedback!

We are moving to ESM on the `stylelint/stylelint` repo:
stylelint/stylelint#5291
},
env: {
es6: true,
node: true,
},
plugins: ["eslint-comments", "jest", "node", "sort-requires"],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] This config is needless because they are automatically set via extends:

  • plugin:eslint-comments/recommended
  • plugin:jest/recommended
  • plugin:node/recommended-module

@@ -111,7 +112,6 @@ module.exports = {
"prefer-rest-params": "error",
"prefer-spread": "error",
"prefer-template": "error",
"sort-requires/sort-requires": "error",
strict: ["error", "global"],
"sort-imports": ["error", { allowSeparatedGroups: true }],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] I've enabled allowSeparatedGroups because the pattern grouping imports by an empty line are common in our code base.

https://eslint.org/docs/rules/sort-imports#allowseparatedgroups

@ybiquitous ybiquitous marked this pull request as ready for review May 28, 2021 02:05
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

I guess we don't need to merge and release this until we're ready to work on stylelint/stylelint#5291?

@ybiquitous
Copy link
Member Author

I guess we don't need to merge and release this until we're ready to work on stylelint/stylelint#5291?

Surely, it seems good to try this unpublished change on stylelint/stylelint as below:

--- a/package.json
+++ b/package.json
@@ -165,7 +165,7 @@
     "benchmark": "^2.1.4",
     "common-tags": "^1.8.0",
     "eslint": "^7.25.0",
-    "eslint-config-stylelint": "^13.1.1",
+    "eslint-config-stylelint": "github:stylelint/eslint-config-stylelint#esm",
     "got": "^11.8.2",
     "husky": "^6.0.0",
     "jest": "^27.0.1",

Perhaps, we may want to update this ESLint configuration as we move forward with our ESM support.

ybiquitous added a commit to stylelint/remark-preset that referenced this pull request Aug 14, 2021
[`remark@14`](https://github.com/remarkjs/remark/releases/tag/14.0.0) has been supported only ESM.
So, this package needs to become ESM to bump up `remark` dependencies.

Here are the installation commands:

```shell
npm install \
  remark-cli@latest \
  remark-frontmatter@latest \
  remark-preset-lint-recommended@latest \
  remark-preset-prettier@latest \
  remark-validate-links@latest

npm install 'github:stylelint/eslint-config-stylelint#esm'
```

Note that this change needs the ESLint configuration for ESM.
See <stylelint/eslint-config-stylelint#133>
This was referenced Sep 14, 2021
@ybiquitous
Copy link
Member Author

@jeddy3 I've created PR #143 to prepare the 14.0.0 release. Can I merge this PR?

@jeddy3
Copy link
Member

jeddy3 commented Sep 14, 2021

Yeah, might as well. Saves us having to release another version when we actually do the ESM migration.

@ybiquitous
Copy link
Member Author

Thanks. I’ll proceed with the release of 14.0.0. 💪🏼

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.

None yet

2 participants