Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

newline-after-import should not apply to inline require's #570

Closed
sindresorhus opened this issue Sep 22, 2016 · 6 comments · Fixed by #579
Closed

newline-after-import should not apply to inline require's #570

sindresorhus opened this issue Sep 22, 2016 · 6 comments · Fixed by #579
Assignees
Milestone

Comments

@sindresorhus
Copy link

Only ones at the top. Or at least provide an option to ignore them.

When I have inline requires, it doesn't really help much to have a newline and it just get's in the way. I've been hit by this many times.

Example: vercel/hyper@8a8fdac#diff-2460efa50e29d32731d41893921a7739R9

@jfmengels
Copy link
Collaborator

Same here, sounds good to me.

Does anyone see the value of keeping this behavior behind an option somehow? I don't. It's not really the purpose of this rule, though this should probably be better explained in the rule documentation.

Some (who like this) might consider this a breaking change (?), so it might be worth shipping with 2.0 (soon) just to be safe.

@sindresorhus
Copy link
Author

I don't see the point in an option.

@benmosher
Copy link
Member

It's not obvious to me what classifies the linked require as inline, but as long as @jfmengels has it under control, I'm cool with whatever you two decide upon.

Breaking changes can certainly pile into v2, I haven't had a chance to publish the first beta yet, anyway.

@jfmengels
Copy link
Collaborator

jfmengels commented Sep 22, 2016

what classifies the linked require as inline

I thought any require whose parent is not Program, but that's obviously not good enough (even var a = require('a'); does not fit).

Actually, I see that we already have this kind of logic in order, so let's just use the same one.
https://github.com/benmosher/eslint-plugin-import/blob/master/src/rules/order.js#L195-L204

@jfmengels jfmengels self-assigned this Sep 22, 2016
@benmosher benmosher added this to the 2.0.0 milestone Sep 23, 2016
@jfmengels
Copy link
Collaborator

@Singles do you have an opinion on this maybe?

@radekbenkel
Copy link
Contributor

AFAIR @lo1tuma implemented most of the code which is being removed in linked PR, so it would be better to ask him :)

benmosher pushed a commit that referenced this issue Sep 26, 2016
* Breaking - newline-after-import: Do not require empty line after inline require (#570)

* Simplify newline-after-import implementation
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants