-
Notifications
You must be signed in to change notification settings - Fork 34
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
ESLint 9, flat config migration #1537
Conversation
🦋 Changeset detectedLatest commit: 57c67df The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
f9c56b1
to
a873c0d
Compare
Wait for downstream to support: typescript-eslint/typescript-eslint#8211 |
b9e81ce
to
b026cb9
Compare
8daf0c5
to
567c233
Compare
07fed65
to
bff669d
Compare
63c0c88
to
c64c909
Compare
3df0fe5
to
c294e56
Compare
Co-authored-by: Sam Chung <[email protected]>
src/cli/lint/internalLints/upgrade/patches/8.2.1/collapseDuplicateMergeKeys.ts
Outdated
Show resolved
Hide resolved
src/cli/lint/internalLints/upgrade/patches/8.2.1/collapseDuplicateMergeKeys.ts
Outdated
Show resolved
Hide resolved
}, | ||
); | ||
|
||
const replaceAllUntilStable = ( |
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.
question: Could we use replaceAll instead?
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.
Unfortunately not. The regex replaces pairs into one; but for cases of three or more in a row, it needs multiple rounds to condense them. I'm open to better approaches, this was a tricky one because I didn't want to parse the file (and end up fully reformatting it when writing it back out), so everything I considered was piles of hacks (including this!)
src/cli/lint/internalLints/upgrade/patches/8.2.1/upgradeESLint.ts
Outdated
Show resolved
Hide resolved
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.
👏
src/cli/lint/internalLints/upgrade/patches/8.2.1/upgradeESLint.ts
Outdated
Show resolved
Hide resolved
'tmp*/', | ||
], | ||
}, | ||
...base.map(({ plugins: { jest: _jest, ...restPlugins } = {}, ...conf }) => ({ |
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.
question: why do we need to ignore the jest plugin from the base?
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.
This could be cleaned up after with an eslint-config-seek change, eslint currently yells if you try to redefine a plugin that's already defined, but if you defined it in a block with files
you can't use it unless it's exactly the same files 😬
@@ -25,7 +25,13 @@ echo '--- pnpm build' | |||
pnpm build | |||
|
|||
echo '--- pnpm pack' | |||
# I'm sure there's a better way to do this | |||
eslint_config_skuba_tar="$(pwd)/packages/eslint-config-skuba/$(cd packages/eslint-config-skuba && pnpm pack | grep -o 'eslint-config-skuba-.*\.tgz')" |
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.
Oh gotcha because we're packing the thing but the eslint config dep is workspace:* it injects the workspace's current version into that so when we install it, it pulls latest. Damn
src/cli/lint/internalLints/upgrade/patches/8.2.1/collapseDuplicateMergeKeys.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Sam Chung <[email protected]>
Co-authored-by: Sam Chung <[email protected]>
Huge PR so if anyone has feedback after the fact happy to loop back! Will endeavour to do some internal snapshot testing and the like on master too. |
FlatESLint
#1157