Add newline-after-import rule#245
Conversation
|
Looks good to me, but is it possible to also support |
|
@jfmengels You mean, you would like it to work for |
|
@Singles Yes :) |
| * Ensure all imports appear before other statements ([`imports-first`]) | ||
| * Report repeated import of the same module in multiple places ([`no-duplicates`]) | ||
| * Report namespace imports ([`no-namespace`]) | ||
| * Enforce new line after import not followed by another import([`newline-after-import`]) |
There was a problem hiding this comment.
Maybe:
Enforce a newline after import statements ([`newline-after-import`])
|
@sindresorhus @jfmengels thank you guys for feedback. Regarding Also I checked Please don't get me wrong - I'm not trying to be cocky or something, it just matter of consistency for me :) EDIT Of course I can add support for |
|
You're right that this project doesn't support From what I understood, this plugin supports rules that can infer something from static analysis using @benmosher Thoughts on supporting |
|
FWIW, For that rule, you may supply an options object like I think it makes sense to support CommonJS to the extent that
I think that is true on both counts in this case. |
|
Now when #247 is here and based on comments from #246 I think it would be nice to solve this as a part of addtion to #247. Just additional option called What do you think? |
|
Tough call. I might enable this rule without worrying about ordering within the imports themselves, so I'm not sure it makes sense rolled into I think this makes sense as you've implemented it for const x = require('x')
require('jqueryui')
var y = require('y')
// ^ line above is enforced
function boringName(someArg) {
return new Error("the worst function ever")
}
let z = require('ugh-mutable-module-reference-ftl')
// ^ another enforced line here
function stahp() {
process.exit(-∞)
}No need to enforce on †: unless @jfmengels or @sindresorhus say otherwise. I don't think it's important for v1 but I'm not going to use it, so what do I know 😅 I guess you could provide the same visitor for both |
|
OK, rebased onto |
| ensureNoForbiddenKeyword(context, node, nextToken, 'import') | ||
| }, | ||
| CallExpression: function(node) { | ||
| if (node.callee.name === "require") { |
There was a problem hiding this comment.
Do you mind using single-quotes here? For consistency with the other strings.
|
LGTM :) |
|
@benmosher no problem, done. |
|
Rebased and added changelog updates. 👍 it's in master, ready for publish via 1.7.0. Not sure when that will be, probably want to get some of @sindresorhus's issues resolved first. |
|
@benmosher It's not very nice of you to say that @sindresorhus has issues :p |
|
It's true. I have a gadzillion issues... |
|
You made me found out I had way more issues than I thought... If it makes you feel better, I have a 3.5 hour trip, and I'll spend at least some of it solving your issues ^^ Edit: No, it's fine, most of them are greenkeeper PRs on a project I have abandoned :D |
newline-after-import (#244)
Reports if there's no new line after last import in group.
Rule Details
Valid:
...whereas here imports will be reported:
When Not To Use It
If you like to visually group module imports with its usage, you don't want to use this rule.