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

Change Request: Support async rules #15394

Open
1 task
coderaiser opened this issue Dec 6, 2021 · 16 comments
Open
1 task

Change Request: Support async rules #15394

coderaiser opened this issue Dec 6, 2021 · 16 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@coderaiser
Copy link
Contributor

coderaiser commented Dec 6, 2021

ESLint version

8.4.0

What problem do you want to solve?

Since ESLint supports async formatters started from v8.4.0, would be great to have support of async plugins:

module.exports = {
    meta: {
        type: "suggestion",

        docs: {
            description: "disallow unnecessary semicolons",
            category: "Possible Errors",
            recommended: true,
            url: "https://eslint.org/docs/rules/no-extra-semi"
        },
        fixable: "code",
        schema: [] // no options
    },
    // would be great if create can return a Promise
    create: function(context) {
        return {
            // callback functions
        };
    }
};

I'm working on 🐊Putout code transformer, and have a plugin for ESLint. The thing is 🐊Putout has plugins which are loaded straight after parsing (depending on options provided by user).
All 🐊Putout plugins are CommonJS, and if they will be converted to ESM would be impossible to use 🐊Putout as a plugin for ESLint, because it supports only synchronous plugins.

This is one of the use-cases, but ESLint plugins can even be written in ESM and be loaded to CommonJS this way:

  create: async function(context) {
       const plugin = await import('./plugin.js'); 
       return plugin;
    }

So this is a big step forward.

What do you think is the correct solution?

Add support of create function that returns Promise similar to the way formatters work.

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response

@coderaiser coderaiser added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Dec 6, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Dec 6, 2021
@nzakas
Copy link
Member

nzakas commented Dec 8, 2021

So what you’re asking for is a way to make rules asynchronous? (Not plugins, plugins are just an object.)

This would require a rewrite of the Linter class, so it’s unlikely to happen anytime soon. Async formatters exist at the ESLint class level, and since that class already had async methods, it was easy to add.

@nzakas nzakas changed the title Change Request: Support async plugins Change Request: Support async rules Dec 8, 2021
@coderaiser
Copy link
Contributor Author

Yes, and I just realized that the best possible solution would be to get AST root node without actual traversing, something like this:

create: async function(context) {
       const ast = context.getAST();
       const code = await modifyCopyOfASTAndReturnString(ast);
       
       context.report({
           fix: () => {
               fixer.replaceTextRange([0, last], code);
           }
       });
    }

This would be the best possible solution. I understand that this is a big change. I can try to help with it. What I want to know is what ESLint team thinks about such a feature?

It will give ability of gradual transition to ESM. It supported by node v12, deno, and all browsers. So this is amazing feature to have :).

@nzakas
Copy link
Member

nzakas commented Dec 9, 2021

Yes, and I just realized that the best possible solution would be to get AST root node without actual traversing

Can you explain what this means? You can already get the AST via context.getSourceCode().ast. Are you looking for something more than that?

As I said, this isn’t a change we would want to do anytime soon due to its complexity. If we need to rewrite Linter, there are a lot of things we need to consider and the team is already working on a couple big projects.

@nzakas nzakas moved this from Needs Triage to Triaging in Triage Dec 9, 2021
@nzakas nzakas moved this from Triaging to Feedback Needed in Triage Dec 9, 2021
@coderaiser
Copy link
Contributor Author

coderaiser commented Dec 9, 2021

You can already get the AST via context.getSourceCode().ast.

This is very good, I didn't know about that (didn't find in documentation).

Are you looking for something more than that?

Right now when I return undefined from create function, I receive an error:

Users/coderaiser/putout/node_modules/eslint/lib/rule-tester/rule-tester.js:329
        throw err;
        ^

TypeError: Cannot convert undefined or null to object
Occurred while linting <input>
    at Function.keys (<anonymous>)
    at /Users/coderaiser/putout/node_modules/eslint/lib/linter/linter.js:1085:16
    at Array.forEach (<anonymous>)
    at runRules (/Users/coderaiser/putout/node_modules/eslint/lib/linter/linter.js:1003:34)
    at Linter._verifyWithoutProcessors (/Users/coderaiser/putout/node_modules/eslint/lib/linter/linter.js:1350:31)

Such code works:

create: async function(context) {
       const ast = context.getAST();
       const code = await modifyCopyOfASTAndReturnString(ast);
       
       context.report({
           fix: () => {
               fixer.replaceTextRange([0, last], code);
           }
       });

       // would be great to not return empty object
       // without this code we have an exception
        return {};
    }

But would be great to have ability to return nothing.

As I said, this isn’t a change we would want to do anytime soon due to its complexity. If we need to rewrite Linter, there are a lot of things we need to consider and the team is already working on a couple big projects.

OK, I understood, not a problem at all. I think we can come back to this in the future, when the time is come.

@github-actions
Copy link

github-actions bot commented Feb 7, 2022

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

@github-actions github-actions bot added the Stale label Feb 7, 2022
@ljharb
Copy link
Sponsor Contributor

ljharb commented Feb 7, 2022

this would still be good to keep open.

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon Stale labels Feb 8, 2022
@nzakas
Copy link
Member

nzakas commented Feb 8, 2022

This was accepted at the last TSC meeting. We just missed adding the label.

@Gautam-Arora24
Copy link
Contributor

Hi! Are we planning to implement this anytime soon?

@nzakas
Copy link
Member

nzakas commented Mar 15, 2022

No. This is on the roadmap but low priority right now.

@chriskrycho
Copy link

@nzakas I'm currently looking at the long-term path for integrating ember-template-lint into ESLint, and ember-template-lint has an async API. This is a pretty key part of Ember's move to embedding templates in JS, so I think I can find folks who are willing to do the work!

However, they would likely need some degree of support to make sure the work they do is useful—we don’t want to waste your time with something that is not ultimately mergeable. Would you folks be open to mentoring or at least providing direction and support if someone were willing to implement it? I know from experience trying to guide someone else is itself super time consuming, so I’m happy to help come up with ways to minimize the overhead.

@nzakas
Copy link
Member

nzakas commented Apr 27, 2022

@chriskrycho definitely! At this point, we aren’t even sure how to begin looking at implementing this, so any help would be appreciated.

In general, the best path forward is to work on an RFC. I understand that’s a big dive into unfamiliar source code, so it can help to swing by #developers in Discord. We can answer any questions there along the way.

@JounQin
Copy link
Contributor

JounQin commented Jul 15, 2022

@nzakas
Copy link
Member

nzakas commented Aug 18, 2023

An update here: We've decided not to pursue async rules until the core rewrite. Because the current core is all synchronous, there's no easy way to transition to asynchronous rules that doesn't involve significant changes. It doesn't make sense to pay that cost twice.

@mmkal
Copy link

mmkal commented Oct 2, 2023

@nzakas out of curiosity - when you say not until the core rewrite, does that mean that the core rewrite will include switching to an async implementation? (Also, is there an issue for the core rewrite that we can watch and/or provide feedback on?)

@nzakas
Copy link
Member

nzakas commented Oct 2, 2023

when you say not until the core rewrite, does that mean that the core rewrite will include switching to an async implementation?

Yes.

Also, is there an issue for the core rewrite that we can watch and/or provide feedback on?

The original discussion is here:
#16557

When we're able to switch gears to the rewrite (sometime between v9.0.0 and v10.0.0), I'll be putting together an RFC with a concrete proposal on what the new core should look like.

@DevDuo224
Copy link

For now, you can try https://github.com/un-ts/synckit

See https://github.com/mdx-js/eslint-mdx/blob/master/packages/eslint-mdx/src/worker.ts for example.

https://www.youtube.com/watch?v=XbpS8UCQa7E&t=190s

made this 👆
for the problem
Check out this if you guys have the same problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Public Roadmap
  
Planned
Status: Blocked
Triage
Feedback Needed
Development

No branches or pull requests

8 participants