-
Notifications
You must be signed in to change notification settings - Fork 5
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 incorrect autofix for nested dependency #299
Conversation
Thanks! Can you fix the lint errors? UPDATE: fixed for you |
lib/dependency-versions.ts
Outdated
fixedVersion | ||
fixedVersion, | ||
// @ts-ignore (@types/edit-json-file not available for 1.7) | ||
{ preservePaths: false } |
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.
Can you add a comment to this line explaining what problem this preservePaths option fixes and how it fixes it?
UPDATE: merging now but maybe you can reply to this comment to explain it better.
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.
Hi @bmish , sorry I didn't explain it very well. If I ran --fix and the dependency name had a slash in, the resulting package.json had a new entry outside of dependencies, which looked like this:
"dependencies: { ... },
"devDependencies": { ... },
"devDependencies.@types/jest": "^27.0.2",
The fix itself is a setting which is supplied to the set-value library, which is a dependency of edit-json-file, that you are using in your tool. You can see the doc for the preservePaths option here: https://github.com/jonschlinkert/set-value#optionspreservepaths - without this setting it assumes that if the property includes a /, it must be a url, so shouldn't be split even on dots. If that makes sense! It took me a while to track down.
Thanks for accepting the PR.
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's pretty obscure, great find!
Thanks! Released in v1.5.0. |
@tamslinn I'm adding the missing TypeScript type for this to @types/edit-json-file so we can clean this up: DefinitelyTyped/DefinitelyTyped#58125 And use this: #313 |
I discovered a bug when fixing dependencies with a slash in the name (e.g. @types/jest) - rather than updating them in place, it was adding properties in the format
"devDependencies.@types/jest": "^27.0.2",
This PR fixes the issue. I had to upgrade the edit-json-file dependency to enable supplying options to
set
. Unfortunately typescript complains because @types/edit-json-file has not been updated to cover the new 3rd argument toset
I also updated a test to cover the bug which is being fixed.