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

Support .mjs configuration files #6905

Closed
ybiquitous opened this issue Jun 5, 2023 · 5 comments · Fixed by #6910
Closed

Support .mjs configuration files #6905

ybiquitous opened this issue Jun 5, 2023 · 5 comments · Fixed by #6910
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules

Comments

@ybiquitous
Copy link
Member

What is the problem you're trying to solve?

Cosmiconfig has supported .mjs files since v8.2. I believe it'd be useful if Stylelint also could support it.

What solution would you like to see?

First, we may need to resolve a problem with loaders. It may not be easy.

loaders: {
'.cjs': (cjsPath, cjsContent) =>
Promise.resolve(defaultLoadersSync['.cjs'](cjsPath, cjsContent)),
'.js': (jsPath, cjsContent) =>
Promise.resolve(defaultLoadersSync['.js'](jsPath, cjsContent)),
},

Then, we must update the CLI help and documentation below:

stylelint/lib/cli.js

Lines 115 to 118 in 2413e8f

- a "stylelint" property in "package.json"
- a ".stylelintrc" file (with or without filename extension:
".json", ".yaml", ".yml", ".js", and ".cjs" are available)
- a "stylelint.config.js" file exporting a JS object

Stylelint expects a configuration object, and looks for one in a:
- `stylelint` property in `package.json`
- `.stylelintrc` file
- `.stylelintrc.{cjs,js,json,yaml,yml}` file
- `stylelint.config.{cjs,js}` file exporting a JS object

See also:

@ybiquitous ybiquitous added the status: needs discussion triage needs further discussion label Jun 5, 2023
@romainmenke
Copy link
Member

First, we may need to resolve a problem with loaders. It may not be easy.

I think this should just work.
I did no change the loader .mjs files so that we wouldn't have to make further changes.

The only hurdle is that we can't write tests for this with jest.

@ybiquitous
Copy link
Member Author

Unfortunately, my try for .mjs fails. Did I miss something?

package.json:

{
  "devDependencies": {
    "stylelint": "^15.7.0"
  },
  "scripts": {
    "test": "stylelint -c config.mjs *.css",
    "test-cjs": "stylelint -c config.cjs *.css"
  }
}

config.mjs:

export default {
  rules: {
    "block-no-empty": true,
  },
};

config.cjs:

module.exports = {
  rules: {
    "block-no-empty": true,
  },
};

test.css:

a{}

Run:

$ npm i
...

$ npm test
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/masafumi.koba/tmp/foo/config.mjs not supported.
Instead change the require of /Users/masafumi.koba/tmp/foo/config.mjs to a dynamic import() which is available in all CommonJS modules.
...

$ npm run test-cjs
test.css
 1:2  ✖  Unexpected empty block  block-no-empty

1 problem (1 error, 0 warnings)

@romainmenke
Copy link
Member

Was that with or without the v8 cache package?

#6907

@alexander-akait
Copy link
Member

We can write test for jest, but it is tricky, we can use vm and run ESM in vm, also we need to run with some flags (not for jest, for Node.js, because it is still experiment), as another workaround we can spawn a process with only loading (i.e. create a small file with our loading API) and test it

@ybiquitous
Copy link
Member Author

Was that with or without the v8 cache package?

Good catch! Using the HEAD version of stylelint can resolve the error: 😅 👍🏼

  "devDependencies": {
-   "stylelint": "^15.7.0"
+   "stylelint": "github:stylelint/stylelint"
  },

We can write test for jest, but it is tricky

It might be ideal for writing tests, but I don't think writing tests for this feature is essential because writing tricky tests for the feature Cosmiconfig provides has very few benefits.


So, we can just update the CLI help and the document to close this issue!

I've labeled the issue as ready to implement. Please consider contributing if you have time.

@ybiquitous ybiquitous added status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules and removed status: needs discussion triage needs further discussion labels Jun 8, 2023
@ybiquitous ybiquitous added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules
Development

Successfully merging a pull request may close this issue.

3 participants