Skip to content
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

[BUG] Unwxpwcted warning "Removed invalid scripts" on publish #6918

Closed
2 tasks done
zdm opened this issue Oct 20, 2023 · 18 comments
Closed
2 tasks done

[BUG] Unwxpwcted warning "Removed invalid scripts" on publish #6918

zdm opened this issue Oct 20, 2023 · 18 comments
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x

Comments

@zdm
Copy link

zdm commented Oct 20, 2023

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Npm shows unexpected warning every time, when I am publishong any of my packahes.

npm publish --dry-run

Result:

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"

...

pachage.json:

{
    "name": "@softvisio/api",
    "version": "2.9.8",
    "description": "API",
    "keywords": [
        "softvisio",
        "api"
    ],
    "homepage": "https://softvisio-node.github.io/api/",
    "bugs": {
        "url": "https://github.com/softvisio-node/api/issues",
        "email": "zdm <zdm@softvisio.net>"
    },
    "repository": {
        "type": "git",
        "url": "git+https://github.com/softvisio-node/api.git"
    },
    "license": "ISC",
    "author": "zdm <zdm@softvisio.net>",
    "type": "module",
    "imports": {
        "#core/*": "@softvisio/core/*",
        "#lib/*": "./lib/*.js"
    },
    "exports": {
        "./*": "./lib/*.js"
    },
    "files": [
        "/bin",
        "/lib",
        "/resources",
        "/src"
    ],
    "dependencies": {
        "linkedom": "^0.15.0",
        "random-location": "^1.1.3"
    },
    "peerDependencies": {
        "@softvisio/core": "^7.0.0"
    }
}

Expected Behavior

No warnings should be desplayed, because package.json has no scripts ptoperty at all.

Steps To Reproduce

See above.

Environment

  • npm: 10.1.0
  • Node.js: v20.8.1
  • OS Name:
  • System Model Name:
  • npm config:
; copy and paste output from `npm config ls` here
@zdm zdm added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Oct 20, 2023
@ljharb
Copy link
Contributor

ljharb commented Oct 20, 2023

If you add an empty one, does it avoid the warning?

@zdm
Copy link
Author

zdm commented Oct 20, 2023

With empty scripts - no warning.

@ljharb
Copy link
Contributor

ljharb commented Oct 20, 2023

so how come your package has no scripts? you're not running any tests?

@zdm
Copy link
Author

zdm commented Oct 20, 2023

I tested it completely before created this issue.
If package.json has no sccrips - it show warning on pusblish.
You can check it yourself.

@ljharb
Copy link
Contributor

ljharb commented Oct 21, 2023

Right, i mean, why don’t you have any scripts?

@zdm
Copy link
Author

zdm commented Oct 21, 2023

Hnn, strange question.
Because most of packages don;t need scripts.

@ljharb
Copy link
Contributor

ljharb commented Oct 21, 2023

Every package needs tests, and the "test" script thus needs to be defined. In other words, virtually zero packages don't need scripts.

@zdm
Copy link
Author

zdm commented Oct 21, 2023

This is other question.
Believe me, 90& of packaged u the world do not use scripts.
In an case scripts are not mandatory., so according to this issure - I thisnk it should be fixed.

@ljharb
Copy link
Contributor

ljharb commented Oct 21, 2023

I don't believe you; I'd guess that 99% of packages in the world do use scripts.

I agree that they're not mandatory and the issue should be fixed, but I also think the reason nobody found this before is because everybody has scripts :-)

@zdm
Copy link
Author

zdm commented Oct 21, 2023

Please, download 100 random packaces from the nom.org and check - do they have scripts.

Listen, this is not related to this issur at all, let's not not wast time.

Scripts are not ,mandatory, so what the sense of this warning, annoy people?
This is not nom business to watch for how user use package.

If you are member of npm dev team and you are decided that this is not a issue - please, close it, bit let's stop crazy discussion.

@zdm
Copy link
Author

zdm commented Oct 21, 2023

lasr words:

If user need scripts - he will use them, if not - not. Without reminders from npm.

npm trying to force user to add scripts, which are not required.

We need to add some fake scripts to make is happy?

The single goal of this warning is to annoy and confuse people.

@wraithgar
Copy link
Member

This is a side effect of how npm has been publishing packuments this whole time.

If you inspect the packages in the registry, you'll see they all have an empty scripts object even if the package.json source does not.

The new logging is a byproduct of our attempts to make sure these two entries are in sync. The decision was made to NOT allow a change change to what's in the registry to minimize potential breaking changes for those who consume these packages.

@vpctorr
Copy link

vpctorr commented Dec 14, 2023

This is a side effect of how npm has been publishing packuments this whole time.

If you inspect the packages in the registry, you'll see they all have an empty scripts object even if the package.json source does not.

The new logging is a byproduct of our attempts to make sure these two entries are in sync. The decision was made to NOT allow a change change to what's in the registry to minimize potential breaking changes for those who consume these packages.

Makes sense to me! However, the warning Removed invalid "scripts" when there was no scripts in the first place is pretty unclear.

@SorinGFS
Copy link

This is a side effect of how npm has been publishing packuments this whole time.
If you inspect the packages in the registry, you'll see they all have an empty scripts object even if the package.json source does not.
The new logging is a byproduct of our attempts to make sure these two entries are in sync. The decision was made to NOT allow a change change to what's in the registry to minimize potential breaking changes for those who consume these packages.

Makes sense to me! However, the warning Removed invalid "scripts" when there was no scripts in the first place is pretty unclear.

Is not just "unclear", is wrong! This is because actually does not remove anything!

@ljharb
Copy link
Contributor

ljharb commented Feb 13, 2024

It doesn’t from package.json, but it does from the packument in the published package.

@mcqj
Copy link

mcqj commented Mar 5, 2024

Explaining the background to the bug is useful for context. However, the end user will not expect a warning message when they have a perfectly valid package.json. Furthermore, a warning message that suggests running npm pkg fix to address the errors, when in fact running that command will not address them creates even more confusion.
The warning message should be removed.

@romainmenke
Copy link

I don't believe you; I'd guess that 99% of packages in the world do use scripts.

We definitely use scripts but we don't see why we should be bloating the npm registry, user node_modules folders and everything in between with several hundred bytes of data that is useless on that end.

We prefer to strip this before publishing.

Some quick maths : we save the interwebs ±120 gigabytes of transfer each week by stripping all scripts from package.json before publishing.

I don't see the point of this warning ...

joscha added a commit to joscha/dnt that referenced this issue Jun 5, 2024
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
timvandermeij added a commit to timvandermeij/pdf.js that referenced this issue 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 issue 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 issue 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.
@l3ender
Copy link

l3ender commented Jul 21, 2024

Still seeing this problem with Node v20.15.1 (NPM 10.7.0). Not quite sure why this issue was closed when the incorrect behavior still exists (summarized well by @mcqj in #6918 (comment)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x
Projects
None yet
Development

No branches or pull requests

9 participants