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

New: Events Model in CLIEngine. Event: "fileVerify" #24

Closed
wants to merge 2 commits into from
Closed

New: Events Model in CLIEngine. Event: "fileVerify" #24

wants to merge 2 commits into from

Conversation

IgorNovozhilov
Copy link

Summary

This proposal describes the implementation of event support to handle the validation of each file separately when using the executeOnFiles() method,
both for synchronous validation and for asynchronous validation in the future, which was proposed earlier: eslint/rfcs #11 - README.md, eslint/rfcs #4 - README.md

Related Issues / PR

eslint/eslint#11733

@IgorNovozhilov IgorNovozhilov changed the title Events Model in CLIEngine. Event: "fileVerify" New: Events Model in CLIEngine. Event: "fileVerify" May 17, 2019
@mysticatea mysticatea added feature Initial Commenting This RFC is in the initial feedback stage and removed triage labels May 18, 2019
@mysticatea
Copy link
Member

Thank you for your proposal.

My first impression is:

  • we should consider it after the specification of asynchronous stuff was determined because we can write the same thing as the event with a few codes on the current ESLint.
  • it should be an option of executeOnFiles() method because it doesn't affect to other methods.

@IgorNovozhilov
Copy link
Author

IgorNovozhilov commented May 18, 2019

If make this an option for executeOnFiles(), we will not be able to use events in synchronous mode, because method will have to return an instance of EventEmitter. And only then start search files.

Roughly so:

executeOnFiles(patterns, options) {
  // do something...
  if (options.useEvents) {
    const events = new EventEmitter()
    onlyAsyncLintingOrSearchMethod(()=>{
      // do something...
      events.emit("fileVerify", result);
    })
    return events
  } else {
    // current behavior
  }

// some file

const events = cli.executeOnFiles(...)

events.on("fileVerify", result => {
  // do something...
})

// oops, we don't know when the end of Linting...
// will need to do one more event to catch the completion of linting

Or do not use events at all and make a callback:

executeOnFiles(patterns, {
  callbackPerFile: true,
  onReport(result) {
    // do something...
  }
})

In favor of my proposal, I would say that if you use events as class properties, then in the future you can expand the list of events by making such events:

  • error - triggered on fatal exception. no config file, config file parse error, other run-time errors
  • report - sending a single problem
  • rawReport - sending raw data a problem - is a origin object of problem from call context.report. (for deeper integration with ESLint rules)
  • loadConfigFile - triggered on load config file from disc. (for deeper integration with ESLint, with the addition of custom configuration pre-validation)

and more... these are the first events that came to mind to improve the integration of eslint in my project.

@mysticatea
Copy link
Member

Your callback example has a syntax error. If it's fixed, it would be:

executeOnFiles(patterns, {
    onReport(result) {
      // do something...
    }
})

I don't see any problem.

@IgorNovozhilov
Copy link
Author

Your callback example has a syntax error. If it's fixed, it would be:

Yes, thanks

@nzakas
Copy link
Member

nzakas commented Mar 31, 2021

Closing, as we won't be moving forward with this. Thanks for submitting the RFC and sorry we lost track of it.

@nzakas nzakas closed this Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Initial Commenting This RFC is in the initial feedback stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants