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

[import/order] Fix incorrect import reordering #1253

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@tihonove
Copy link
Contributor

tihonove commented Jan 3, 2019

Closes #1252

tihonove added some commits Jan 3, 2019

Fix import/order fixer
Reordering import statement to line below ignores uncrossable statements
Fix #1252
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 68664ed on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

9 similar comments
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.292% when pulling 9426cfb on tihonove:master into 1cd82eb on benmosher:master.

@tihonove

This comment has been minimized.

Copy link
Contributor

tihonove commented Jan 3, 2019

I have no idea why ci builds failed. Seams like build for eslint5 broken.

code: `
const local1 = require('./local1');
const local2 = require('./local2');
const local3 = require('./local3');

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Collaborator

All of these new tests start with each group already ordered; can we unordered them, and show that the autofixer properly orders them?

This comment has been minimized.

@tihonove

tihonove Jan 3, 2019

Contributor

@ljharb Great! I wrote requested tests and found some cases that autofixer dont order imports inside group. I inspected code and find out that ability to reorder imports inside group requires great rework of rule code (another issue?).

This PR just fix a bug in current algorithm of rule. I suggest use this PR as quick fix, and create another (less critical) issue to fix sorting inside groups.

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Collaborator

In that case I'd still like to see the test cases show what it currently does when it starts out out of order; perhaps with additional cases instead of changing these?

This comment has been minimized.

@tihonove

tihonove Jan 3, 2019

Contributor

Added few tests with unordered groups around function call, including cases with not fully reordered groups after applying autofix

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