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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants