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: use eslint-config-ipfs import sorting #1178

Closed
wants to merge 4 commits into from

Conversation

SgtPooki
Copy link
Member

This PR shows an example of what changes ipfs/eslint-config-ipfs#126
would enforce. All of this these changes (except package.json) were done
by running npm run lint -- --fix.

CALLOUTS:

  1. eslint was failing with locally linked package on missing plugins.
  2. lint fix was called twice, with the first run failing due to an empty
    line the first fix runthrough didn't fix.
  3. tsc seems to be failing for aegir

This PR shows an example of what changes ipfs/eslint-config-ipfs#126
would enforce. All of this these changes (except package.json) were done
by running `npm run lint -- --fix`.

CALLOUTS:
1. eslint was failing with locally linked package on missing plugins.
1. lint fix was called twice, with the first run failing due to an empty
line the first fix runthrough didn't fix.
1. tsc seems to be failing for aegir
@achingbrain
Copy link
Member

tsc seems to be failing for aegir

It's failing because the automated reordering of imports didn't also move the associated // @ts-expect-error comments from above imports of modules that don't export types.

E.g. this used to work:

// @ts-expect-error foo-no-types has no types
import foo from 'foo-no-types'
import bar from 'bar-has-types'

then with the changes in ipfs/eslint-config-ipfs#126 aegir lint --fix turns it into this:

// @ts-expect-error foo-no-types has no types
import bar from 'bar-has-types'
import foo from 'foo-no-types'

so it explodes because the @ts-expect-error is no longer on the line above the import of the module that has no types.

Copy link
Contributor

@whizzzkid whizzzkid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, applying this rule doesn't seem to move associated comments in utils/echo-server.js

https://github.com/ipfs/aegir/actions/runs/4057685025/jobs/6983724772

utils/echo-server.js Show resolved Hide resolved
utils/echo-server.js Outdated Show resolved Hide resolved
achingbrain added a commit to ipfs/eslint-config-ipfs that referenced this pull request Apr 5, 2023
Lint import sorting, and support auto-fixing of import order. see ipfs/aegir#1178 for an example

---------

Co-authored-by: Alex Potsides <[email protected]>
achingbrain added a commit to ipfs/eslint-config-ipfs that referenced this pull request Apr 5, 2023
Lint import sorting, and support auto-fixing of import order. see ipfs/aegir#1178 for an example

---------

Co-authored-by: Alex Potsides <[email protected]>
@achingbrain
Copy link
Member

Superseded by #1229

@achingbrain achingbrain deleted the chore/import-sorting branch April 27, 2023 11:13
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