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
Update: addresses #9947 #10653
Update: addresses #9947 #10653
Conversation
Format newly generated .eslintrc.js file before it is saved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
For the test, one idea would be to use the mock-fs
package, which is also used in a few other tests elsewhere.
@@ -18,7 +18,8 @@ const fs = require("fs"), | |||
pathIsInside = require("path-is-inside"), | |||
stripComments = require("strip-json-comments"), | |||
stringify = require("json-stable-stringify-without-jsonify"), | |||
requireUncached = require("require-uncached"); | |||
requireUncached = require("require-uncached"), | |||
Linter = require("../linter.js"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure using Linter
here will work correctly when the generated config contains something like extends
(i.e. when the user says they want to follow a popular styleguide). The second argument of Linter#verifyAndFix
is supposed to be a "fully-resolved" config (i.e. it just has rules, without depending on any external plugins or extends
clauses).
It might be better to use the CLIEngine#executeOnText
API instead. For example, maybe something like this would work:
const results = new CLIEngine({ useEslintrc: false, ignore: false, baseConfig: config, fix: true })
.executeOnText(content, ".eslintrc.js")
// ... extract output from results
@mbildner Friendly ping! Anything we can do to help you land this? |
Closing, as there has been no activity 90 days. |
Format newly generated .eslintrc.js file before it is saved.
What is the purpose of this pull request? (put an "X" next to item)
[x] Other, please explain:
Slight improvement in the cli by addressing #9947
What changes did you make? (Give an overview)
When a user creates a new
.eslintrc.js
file viaeslint --init
, the resulting file conforms to the rules generated by the cli.Is there anything you'd like reviewers to focus on?
I could not figure out how to feature test this.
It is tested by implication: a unit test confirms that when a config file is written, if that file is a
.js
config file, and rules exist in the config that is being saved, then those rules are obeyed in the resulting file.Also note: this change uses the
Linter
class under the hood instead of theCLIEngine
. The latter cannot be imported by theConfigFile
class. I tried a little and decided it made more sense to reach straight to theLinter
instead.