Skip to content

chore(repo): update eslint and remove workaround #981

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 11 commits into from
Closed

chore(repo): update eslint and remove workaround #981

wants to merge 11 commits into from

Conversation

tjenkinson
Copy link
Member

@tjenkinson tjenkinson commented Aug 21, 2021 •
edited
Loading

Rollup Plugin Name: NA

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers: closes #977

Description

See the linked ticket. With the eslint and prettier update we shouldn't need the workaround this PR removes anymore which was to handle an eslint crash if there was a folder called "package.json".

@tjenkinson tjenkinson changed the title Update eslint remove workaround chore: update eslint and remove workaround Aug 21, 2021
github-actions[bot]
github-actions bot reviewed Aug 21, 2021
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your PR title doesn't match the required format. The title should be in the conventional commit (https://www.conventionalcommits.org/en/v1.0.0-beta.4/) format. e.g.

chore(plugin-name): add pr title workflow

@tjenkinson tjenkinson force-pushed the update-eslint-remove-workaround branch from 94cb301 to 9e70b29 Compare August 21, 2021 12:03
@tjenkinson tjenkinson changed the title chore: update eslint and remove workaround chore(repo): update eslint and remove workaround Aug 21, 2021
@tjenkinson
Copy link
Member Author

tjenkinson commented Aug 21, 2021 •
edited
Loading

Ugh this is a can of worms. Now it's looking like the same bug exists in prettier /0\

Oops! Something went wrong! :(

ESLint: 7.32.0

Error: EISDIR: illegal operation on a directory, read
Occurred while linting /Users/tomjenkinson/Documents/GitHub/plugins/packages/node-resolve/test/fixtures/package-json-in-path/package.json/main.js:1
    at Object.readSync (node:fs:625:3)
    at tryReadSync (node:fs:390:20)
    at Object.readFileSync (node:fs:427:19)
    at readFileSync (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9675:35)
    at ExplorerSync.loadSearchPlaceSync (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9993:51)
    at ExplorerSync.searchDirectorySync (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9979:34)
    at run (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9959:29)
    at cacheWrapperSync (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9715:20)
    at ExplorerSync.searchFromDirectorySync (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9971:52)
    at run (/Users/tomjenkinson/Documents/GitHub/plugins/node_modules/.pnpm/[email protected]/node_modules/prettier/third-party.js:9963:23)

This was referenced Aug 21, 2021
Merged
Closed
Merged
@tjenkinson
Copy link
Member Author

blocked on prettier/prettier#11382 going into stable version

@tjenkinson tjenkinson force-pushed the update-eslint-remove-workaround branch from 31989a7 to d7a6f4a Compare September 9, 2021 19:41
@tjenkinson
Copy link
Member Author

@shellscape any idea why tests are failing on CI?

pnpm ci:coverage --filter "...[master]" is working fine for me locally on node v16.9.0

Somewhere in the logs I see packages/pluginutils ci:coverage: ✖ Timed out while running tests but there isn't much more to go on :/

@tjenkinson
Copy link
Member Author

When I run run pnpm test -r on master some tests fail actually so wonder if it might be related to that?

@shellscape shellscape force-pushed the master branch 4 times, most recently from b353836 to 3038271 Compare October 21, 2022 19:01
@shellscape
Copy link
Collaborator

Closing this one as complete. @lukastaegert did a bunch of work to resolve this.

@shellscape shellscape closed this Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@shellscape shellscape Awaiting requested review from shellscape shellscape will be requested when the pull request is marked ready for review shellscape is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove .eslintignore workaround and update eslint to at least 7.31.0
2 participants
@tjenkinson @shellscape