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 custom formatter for ESM #7267

Closed
Tracked by #6930
ybiquitous opened this issue Oct 20, 2023 · 2 comments · Fixed by #7343
Closed
Tracked by #6930

Support custom formatter for ESM #7267

ybiquitous opened this issue Oct 20, 2023 · 2 comments · Fixed by #7343
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?

Currently, we support only a custom formatter written in CommonJS (CJS). But, it would be useful if people could also specify a custom formatter in ESM.

stylelint/lib/cli.mjs

Lines 396 to 400 in 958bf36

if (cli.flags.customFormatter) {
const customFormatter = resolveCustomFormatter(cli.flags.customFormatter);
formatter = require(customFormatter);
}

Related to #6930

What solution would you like to see?

I suggest supporting a custom formatter written in ESM. This means we need to change the current requiring logic.

@ybiquitous ybiquitous added the status: needs discussion triage needs further discussion label Oct 20, 2023
@mattxwang
Copy link
Member

This seems like a reasonable idea to me! From my understanding, it wouldn't be too complicated right (?) - as we can synchronously load CJS from ESM code?

(perhaps we could also look to the community and see which formatters are popular, which could help us guide the API)

@ybiquitous
Copy link
Member Author

I guess we could use dynamic import (aka import()), which is available both ESM and CJS modules as the Node.js document says:

Dynamic import() is supported in both CommonJS and ES modules. In CommonJS modules it can be used to load ES modules.

We must change the code using Promise when adopting this approach since import() returns a Promise. But this should not be problematic because the cli.mjs's main function is already an async function.

@ybiquitous ybiquitous mentioned this issue Nov 13, 2023
10 tasks
@jeddy3 jeddy3 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 Nov 13, 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 Nov 15, 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