-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: add fromContent method to set contents directly #96
Conversation
e1cf483
to
b506ba0
Compare
@@ -167,6 +167,12 @@ class PackageJson { | |||
return this | |||
} | |||
|
|||
fromContent (data) { |
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 think this method was ill conceived. In my head it was the implementation that this PR is now providing.
Having it public was probably a mistake, as it should probably also have #canSave
set to false if used directly (and load
should not use it directly).
That's neither here nor there for this PR but those are the thoughts that this PR makes me have.
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.
fromComment
seems to have the same issue. I think that for how we use this it's fine, but some of these public methods could be used in a way that gives weird behavior when folks try to save.
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 is good
🤖 I have created a release *beep* *boop* --- ## [5.1.0](v5.0.3...v5.1.0) (2024-04-22) ### Features * [`17788d0`](17788d0) [#96](#96) add fromContent method to set contents directly (#96) (@lukekarrys) ### Chores * [`9597b84`](9597b84) [#94](#94) postinstall for dependabot template-oss PR (@lukekarrys) * [`5e20e64`](5e20e64) [#94](#94) bump @npmcli/template-oss from 4.21.3 to 4.21.4 (@dependabot[bot]) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Currently to replace
read-package-json-fast
inpacote
we already have a manifest that needs normalizing. The only way to do that currently is to pass in a string and re-JSON.parse it which is a waste. This allows for content to be set directly.