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

Breaking: remove `source` property from individual eslint.verify messages #7358

Closed
vitorbal opened this Issue Oct 13, 2016 · 3 comments

Comments

Projects
3 participants
@vitorbal
Member

vitorbal commented Oct 13, 2016

This is a follow-up of #7098.

We should remove the source property from individual linting error messages that eslint.verify() outputs, in order to avoid duplicate information in the results.

For more information, please see this comment.

This is a breaking change, and as agreed, we plan on introducing it only for 5.0.0. Justification:

[...] as we're planning on having 4.0.0 before the end of the year, it's probably best to keep the source property for each individual message until 5.0.0, to give people time to adjust their formatters.

@vitorbal

This comment has been minimized.

Member

vitorbal commented Oct 13, 2016

@nzakas marking this as accepted as we've already agreed on it in #7098. Hope that's okay!

@vitorbal vitorbal added this to the v5.0.0 milestone Oct 13, 2016

@nzakas

This comment has been minimized.

Member

nzakas commented Oct 15, 2016

Yup!

@not-an-aardvark not-an-aardvark added this to Accepted in v5.0.0 Oct 31, 2017

not-an-aardvark added a commit that referenced this issue Feb 23, 2018

@not-an-aardvark not-an-aardvark moved this from Accepted, ready to implement to Ready to merge in v5.0.0 Feb 23, 2018

not-an-aardvark added a commit that referenced this issue Feb 27, 2018

not-an-aardvark added a commit that referenced this issue Mar 3, 2018

v5.0.0 automation moved this from Ready to merge to Done Mar 22, 2018

not-an-aardvark added a commit that referenced this issue Mar 30, 2018

@not-an-aardvark not-an-aardvark moved this from Ready to Implement to Done in Core Roadmap May 7, 2018

@bdkjones

This comment has been minimized.

bdkjones commented Sep 18, 2018

I can adapt to this change, but I'm a little curious about why it was made. In what context is ESLint run by something that's unaware of the file that's being linted? In other words, why is it necessary to return the full text of the file in the linting result if the thing that called ESLint already knows the file and could, if desired, read it and then find the line(s) it wants to display for each error (if it wanted to display more than just the line where the error occurred)?

If there are such contexts, wouldn't the filePath property on the returned object provide a way to retrieve the entire contents of the linted file, should a formatter desire that?

The new approach is more memory-intensive and slightly less performant, as I now have to manually find and retrieve the line to display for each error once ESLint is done running.

@eslint eslint bot locked and limited conversation to collaborators Sep 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.