-
Notifications
You must be signed in to change notification settings - Fork 2.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
fix(core): remove .eslintrc double extends update on move #6765
fix(core): remove .eslintrc double extends update on move #6765
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/nrwl/nx-dev/8R4MHmdgKV3p8gLskkxjC6Sac98o [Deployment for 5f55780 canceled] |
Nx Cloud ReportCI ran the following commands for commit 5f55780. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch Sent with 💌 from NxCloud. |
@@ -41,28 +23,14 @@ export function updateEslintrcJson( | |||
schema: NormalizedSchema, | |||
project: ProjectConfiguration | |||
) { | |||
tree['recordedChanges']; |
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 doesn't seem necessary.
updateJson<PartialEsLintRcJson>(tree, eslintRcPath, (eslintRcJson) => { | ||
if (typeof eslintRcJson.extends === 'string') { |
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 feel like we should keep it in here and take it out of updateRootProjectFIles
since this is more specific.
And we don't need 2 places that both update the `eslintrc.json
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.
That was my initial take, but the other one is more generic. I would have to explicitly exclude it from root files.
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'll move the logic here then
2c79969
to
5f55780
Compare
* fix(core): remove eslintrc double extends update on move * fix(core): move eslintrc move logic back to eslint update
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Both
updateProjectRootFiles
andupdateEslintrcJson
modify relative paths in the.eslintrc.json
'sextends
, causing change to be applied twice.Expected Behavior
Only
updateProjectRootFiles
should modify relative paths in the.eslintrc.json
.Related Issue(s)
Fixes #