[BUG] ``npm run`` inside package.json scripts are not aware of workspace context

ferferga commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

DISCLAIMER: I've searched for similar issues before opening this one, but none of them seemed to be exactly what I'm experiencing, as there seems to have been a lot of regressions in workspaces between 8.3.x and 8.5.x versions. If this issue is a duplicate, feel free to close it and sorry for the inconveniences

This issue exists in the latest npm version

[X] I am using the latest npm

Context

I have the following workspaces, which are folders in the root directory of the monorepo (there is no nesting and nothing awakward as I've seen described in other issues)

"workspaces": [
    "frontend",
    "tauri"
  ],

The frontend workspace has the following script:

"lint": "npm run lint:js && npm run lint:style",
"lint:js": "eslint --ext .ts,.js,.json,.vue .",
"lint:style": "stylelint **/*.{vue,css} --ignore-path .gitignore",

The lint one is the one that matters here, the others are just demoed here to show that they're declared and exist.

Additionally, there is a .npmrc file in the root of the monorepo where I set to always default to the frontend workspace (full npm config ls attached below anyway):

workspace=frontend

Current Behavior

With NodeJS 16.14.0 (npm 8.3.1), executing npm run lint in the root of the project executed the command successfully, passing the workspace context correctly to the npm run lint:js && npm run lint:style commands. However, after upgrading to NodeJS 16.14.2 (npm 8.5.0), the workspace context is not recognised, and the following console output is printed:

> @jellyfin-vue/frontend@0.0.0 lint
> npm run lint:js && npm run lint:style

npm ERR! No workspaces found:
npm ERR!   --workspace=frontend

Manually upgrading to npm 8.5.5 has no difference, so it's also affected by the bug.

Expected Behavior

npm run inside package.json scripts should be aware of the workspace context of the invoking command

Steps To Reproduce

Both with npm 8.5.0 and npm 8.5.5:

Clone https://github.com/jellyfin/jellyfin-vue
Run npm i to install dependencies
Run npm run lint
See error...

In order to reproduce a working state, just downgrade NodeJS to 16.14.0 or npm to 8.3.1

Environment

npm: Tested in 8.5.0 and 8.5.5
Node.js: 16.14.2
OS Name: Debian Bullseye (on Window's WSL)
System Model Name:

npm config:


npm WARN config This command does not support workspaces.
; "project" config from /mnt/c/Users/Fernando/jellyfin-vue/.npmrc

fetch-retries = 15 fetch-retry-maxtimeout = 9900000 fetch-retry-mintimeout = 5900000 fetch-timeout = 900000 fund = false lockfile-version = "3" maxsockets = 2 save-exact = true save-prefix = "" workspace = ["frontend"]

; node bin location = /usr/bin/node ; cwd = /mnt/c/Users/Fernando/jellyfin-vue ; HOME = /home/ferferga ; Run npm config ls -l to show all defaults.

fritzy commented 2 years ago

This works in v8.5.2, but fails in v8.5.5.

jellyfin-vue on  master [!] via ⬢ v17.4.0 took 4s 
❯ npm run lint    

> @jellyfin-vue/frontend@0.0.0 lint
> npm run lint:js && npm run lint:style

npm ERR! No workspaces found:
npm ERR!   --workspace=frontend

fritzy commented 2 years ago

It looks like the workspace is being applied twice.

npm ERR! Lifecycle script `lint` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @jellyfin-vue/frontend@0.0.0 
npm ERR!   at location: /Users/fritzy/projects/scratchpad/jellyfin-vue/frontend

ferferga commented 2 years ago

It looks like the workspace is being applied twice.

@fritzy Perhaps this is due to the fact that I have the workspace defined in .npmrc?

Either way, this was working fine in 8.3.1 as I state in the issue description.

ferferga commented 2 years ago

Hello, is there any progress in this? I would like to know if this might take longer than expected, so I can take mitigation steps in our workflows until a patch is done.

In npm 8.8.0 the bug is still reproducible.

dannymichel commented 2 years ago

It looks like the workspace is being applied twice.

npm ERR! Lifecycle script `lint` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @jellyfin-vue/frontend@0.0.0 
npm ERR!   at location: /Users/fritzy/projects/scratchpad/jellyfin-vue/frontend

Is there a workaround for this to run vue?

wraithgar commented 10 months ago

I can't reproduce this in isolation in npm 9 or 10

setup:

$ npm init -y
$ npm init -y -w frontend
$ npm pkg set scripts.lint="echo lint" -w frontend

npm 10:

~/D/n/s/run $ npm run lint

> frontend@1.0.0 lint
> echo lint

lint
~/D/n/s/run $ cd frontend/
~/D/n/s/r/frontend $ npm run lint

> frontend@1.0.0 lint
> echo lint

lint
~/D/n/s/r/frontend $ npm -v
10.2.3
~/D/n/s/r/frontend $ npm run lint

> frontend@1.0.0 lint
> echo lint

lint

npm 9:

~/D/n/s/run $ npx npm@next-9 run lint

> frontend@1.0.0 lint
> echo lint

lint
~/D/n/s/run $ cd frontend/
~/D/n/s/r/frontend $ npx npm@next-9 run lint

> frontend@1.0.0 lint
> echo lint

lint
~/D/n/s/r/frontend $ npx npm@next-9 -v
9.9.0
~/D/n/s/r/frontend $ npx npm@next-9 run lint

> frontend@1.0.0 lint
> echo lint

lint

wraithgar commented 10 months ago

In the repo itself it seems to also work in npm 10

~/D/n/s/r/j/jellyfin-vue (master|✔) $ npm run lint -w frontend

> @jellyfin-vue/frontend@0.3.1 lint
> eslint . --ignore-path ../.gitignore --max-warnings=0 --cache --cache-location ../node_modules/.cache/eslint

ferferga commented 10 months ago

@wraithgar I can confirm you that it's not fixed and this issue should be reopened.

The given repository has evolved a lot since then (in part due to this issue) and now the workspace context needs to be manually specified (or cd into the frontend folder). just like you did.

Before, the workspace was defaulted to frontend in .npmrc, like this:

https://github.com/jellyfin/jellyfin-vue/blob/c64623e1d7abdce5685962d809efdae8518ef9c4/.npmrc#L4

So npm commands previously ran everywhere in the repo without the -w argument was defaulted to that workspace (which is where the main work gets done) and commands with it (i.e npm run build -w tauri) ran in that workspace (we wanted workspaces to be opt-out by default, instead of opt-in, basically).

I created a frozen repository in that specific commit (the last with the old folder structure) to ease the reproduction.

Steps to reproduce

Clone repository and cd into it.
Run npm run lint. The command will successfully run from the repository's root folder (since it takes frontend as a workspace.
Run npm run check. It will fail since that command runs other npm scripts inside it.

I also created a codespaces branch where npm gets updated automatically to @latest on creation if you prefer to reproduce in a devcontainer

wraithgar commented 10 months ago

npm run does work in other workspace contexts. There's something else going on in this repo that will need more investigation

~/D/n/s/workspaces $ npm pkg get scripts -w workspace-a
{
  "workspace-a": {
    "foo": "npm run bar",
    "bar": "echo hello"
  }
}

~/D/n/s/workspaces $ npm run foo -w workspace-a

> workspace-a@2.25.0 foo
> npm run bar

> workspace-a@2.25.0 bar
> echo hello

hello

~/D/n/s/workspaces $ npm config set workspace=workspace-a --location=project
~/D/n/s/workspaces $ npm run foo

> workspace-a@2.25.0 foo
> npm run bar

> workspace-a@2.25.0 bar
> echo hello

hello

Even if the subcommand is calling out to an installed package it works

~/D/n/s/workspaces $ npm run foo -w workspace-a

> workspace-a@2.25.0 foo
> npm run bar

> workspace-a@2.25.0 bar
> eslint .

Oops! Something went wrong! :(

ESLint: 8.53.0

ESLint couldn't find a configuration file.

$ npm -v
9.9.1

I was on the latest 9 here but 10 behaved the same way.

wraithgar commented 10 months ago

Found it. It's the fact that you have a scoped package. --workspace takes a path OR a package name. You're setting it to a path which becomes incorrect when it is run inside the secondary workspace context.

For this to work as you intended you need to set workspace to the package name

~/D/n/s/npm-4618-reproduction (master|✚2) $ git diff
diff --git a/.npmrc b/.npmrc
index 08930b64..bda85919 100644
--- a/.npmrc
+++ b/.npmrc
@@ -1,7 +1,7 @@
 fund=false
 lockfile-version=3
 save-exact=true
-workspace=frontend
+workspace=@jellyfin-vue/frontend
 engine-strict=true

~/D/n/s/npm-4618-reproduction (master|✚1) $ npm run check

> @jellyfin-vue/frontend@0.3.1 check
> npm run lint && npm run typecheck

> @jellyfin-vue/frontend@0.3.1 lint
> eslint . --ignore-path ../.gitignore --max-warnings=0 --cache --cache-location node_modules/.cache

npm / cli