fix: missing scripts section
#414
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Without it, npm complains about it: ``` npm WARN publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors. npm WARN publish errors corrected: npm WARN publish Removed invalid "scripts" ``` via npm/cli#6918 it looks like this is not going to be fixed, thus the change, which removes the warnings
|
|
joscha
commented
Jun 5, 2024
timvandermeij
added a commit
to timvandermeij/pdf.js
that referenced
this pull request
Jun 30, 2024
This commit removes the following warning logs from the `npm publish` output: ``` npm warn publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors. npm warn publish errors corrected: npm warn publish Removed invalid "scripts" npm warn publish "repository.url" was normalized to "git+https://github.com/mozilla/pdfjs-dist.git" ``` For the "scripts" section it turns out that if the package doesn't have any scripts it's expected to explicitly set it to an empty object; refer to npm/cli#6918 and denoland/dnt#414. For the "repository.url" section it turns out that the URL must point to a Git URL that doesn't resolve to an HTML page in the browser; refer to https://docs.npmjs.com/cli/v10/configuring-npm/package-json#repository.
timvandermeij
added a commit
to timvandermeij/pdf.js
that referenced
this pull request
Jun 30, 2024
This commit removes the following warnings from the `npm publish` output: ``` npm warn publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors. npm warn publish errors corrected: npm warn publish Removed invalid "scripts" npm warn publish "repository.url" was normalized to "git+https://github.com/mozilla/pdfjs-dist.git" ``` For the "scripts" section it turns out that if the package doesn't have any scripts it's expected to explicitly set it to an empty object; refer to npm/cli#6918 and denoland/dnt#414.
timvandermeij
added a commit
to timvandermeij/pdf.js
that referenced
this pull request
Jun 30, 2024
This commit removes the following warnings from the `npm publish` output: ``` npm warn publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors. npm warn publish errors corrected: npm warn publish Removed invalid "scripts" npm warn publish "repository.url" was normalized to "git+https://github.com/mozilla/pdfjs-dist.git" ``` For the "scripts" section it turns out that if the package doesn't have any scripts it's expected to explicitly set it to an empty object; refer to npm/cli#6918 and denoland/dnt#414.
dsherret
reviewed
Aug 13, 2024
joscha
changed the title
scripts sectionscripts section
joscha
commented
Aug 14, 2024
| @@ -94,7 +94,7 @@ export function getPackageJson({ | |||
| // override with specified scripts | |||
| ...(packageJsonObj.scripts ?? {}), | |||
| }) | |||
| : packageJsonObj.scripts; | |||
| : packageJsonObj.scripts ?? {}; | |||
There was a problem hiding this comment.
theoretically we could remove this one here, as it gets removed further down by deleteEmptyKeys, however the other branch of the condition has the fallback and we want this to also be there if the code below changes at a later point in time.
joscha
commented
Aug 14, 2024
| @@ -112,6 +112,7 @@ export function getPackageJson({ | |||
| ...mainExport, | |||
| ...binaryExport, | |||
| ...packageJsonObj, | |||
| scripts: {}, | |||
There was a problem hiding this comment.
this change is needed in addition, as the splat from deleteEmptyKeys won't have the scripts key if it is an empty object.
|
friendly ping @dsherret :) |
dsherret
approved these changes
Nov 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Without it, npm complains about it:
via npm/cli#6918 it looks like this is not going to be fixed, thus the change, which removes the warnings