-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Yarn 4 upgrade fixes #21818
base: main
Are you sure you want to change the base?
Yarn 4 upgrade fixes #21818
Conversation
no issue - Yarn v4 introduces some breaking changes to the way flags work, so we had to change the way we install them on CI.
This reverts commit 36a85fb.
I've reached out to Josh Goldberg (Commiter of ESLint + Lead Maintainer of TypeScript-ESlint) to see if we can figure out the issues with the upstream eslint-plugin-ghost code
# Conflicts: # .gitignore # ghost/bookshelf-repository/package.json # nx.json # package.json # yarn.lock
OK, |
@@ -116,17 +122,27 @@ | |||
}, | |||
"devDependencies": { | |||
"@actions/core": "1.11.1", | |||
"@kapouer/eslint-plugin-no-return-in-loop": "1.0.0", | |||
"@typescript-eslint/eslint-plugin": "6.9.1", | |||
"@typescript-eslint/parser": "6.9.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eslint-plugin-ghost
has specific versions of tseslint packages, and is actually on the latest 8.17.0:
Having a different version here will give you two versions. eslint/eslint#15681 might be the issue?
I'm not super confident this is what you're seeing and haven't set all this stuff up locally to try. If this isn't the right fix, let me know and I can poke deeper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I see the confusion this is my bad.
A few things to note:
- The
TryGhost/Ghost
repo is using an older version ofeslint-plugin-ghost
that includes these specific versions - I'm hoping to remove the need for the "install in the monorepo"
peerDeps
-style dep sharing.
Instead, I'd love to npm i eslint-plugin-ghost
and have it handle the versions of @typescript-eslint
and such for us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative, are there blockers to just using flat config? 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, what folks normally do is require.resolve("@typescript-eslint/eslint-plugin)
inside the plugin. That way it resolves to the specific string path within your node_modules/eslint-plugin/ghost/node_modules/
(if that's not deduplicated to something higher up). I'll try that out now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I, ah, don't know why this isn't working: https://github.com/JoshuaKGoldberg/eslint-plugin-ghost/tree/require-resolve.
Steps to repro what I thought would fix things:
- Check out that branch locally
- Link the
Ghost
repo to that branch with"eslint-plugin-ghost": "file:../eslint-plugin-ghost"
in thepackage.json
'sdependencies
- Remove all
parser: '@typescript-eslint/parser'
lines from ESLint config files in theGhost
repo
I do see console logs from the local package. But:
Oops! Something went wrong! :(
ESLint: 8.44.0
ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin".
(The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/Users/josh/repos/Ghost/ghost/nql-filter-expansions".)
It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
npm install @typescript-eslint/eslint-plugin@latest --save-dev
The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in ".eslintrc.js » plugin:ghost/ts".
If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
21/95 targets fail in yarn lint
. 🤔
This PR gets Yarn 4 (Berry) into a working state, based off of the work done by @ronaldlangeveld in #21292
It does this by: