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

chore(eslint): optimize imports #1315

Closed
wants to merge 1 commit into from

Conversation

LitoMore
Copy link
Member

@LitoMore LitoMore commented Jan 19, 2022

Changes

I moved all unassigned imports (CSS/Less/MP3) to the bottom for a better visual.

When to disable it?

And I am using eslint-disable-next-line import/order to allow our unassigned sibling CSS/Less imports after those imports from package. Just to avoid some style override issues. Here is an example:

import 'videojs/example.css';
// eslint-disable-next-line import/order
import './style.less';

I don't want to take too much effort on pathGroups. Using eslint-disable-next-line import/order is easilier.

Tips

Use the split diff view for better visual.

image

Plan

I'm going to enable ESLint for all workspaces and root files in another PR.

@BlackHole1
Copy link
Collaborator

Wow. Thank you for the PR! This is something I've always wanted to do! 🤩

Please wait a moment, I need to test this RP locally

@BlackHole1
Copy link
Collaborator

@LitoMore Can you resolve the conflict?

@LitoMore
Copy link
Member Author

@BlackHole1 Conflict has been resolved.

Please hold on other PRs until this one get merged.

@LitoMore LitoMore changed the title chore(eslint): add sort-imports and import/order chore(eslint): add sort-imports and eslint-plugin-import Jan 21, 2022
@LitoMore LitoMore changed the title chore(eslint): add sort-imports and eslint-plugin-import chore(eslint): optimize imports Jan 21, 2022
@crimx
Copy link
Member

crimx commented Jan 24, 2022

@LitoMore
Copy link
Member Author

@crimx You can create another PR and try to replace them (based on this PR). I'm not going to take too much effort into it.

@BlackHole1
Copy link
Collaborator

I will be responsible for the revision of this PR as @LitoMore does not have the extra time to do it

@LitoMore
Copy link
Member Author

We can merge. Then change to eslint-plugin-simple-import-sort.

@BlackHole1
Copy link
Collaborator

Hey @LitoMore 👋
The other day, I communicated with @crimx about this PR.
Currently this PR cannot be merged immediately as this PR automatically modifies the order in which some style files are introduced, which may result in style errors.
So to be on the safe side, we should address this issue in the current PR rather than opening a new one to address it.🙇

@LitoMore
Copy link
Member Author

OK. Closing in favor of the new one.

@LitoMore LitoMore closed this Jan 24, 2022
@LitoMore LitoMore deleted the import-sort-and-order branch September 22, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants