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.

Sign up for GitHub

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

Jump to bottom

[BUG] install script does not run in workspace in 8.5.4 #4552

Closed
2 tasks done
perrin4869 opened this issue Mar 11, 2022 · 4 comments · Fixed by #4875
Closed
2 tasks done

[BUG] install script does not run in workspace in 8.5.4 #4552

perrin4869 opened this issue Mar 11, 2022 · 4 comments · Fixed by #4875
Assignees
@ruyadorno
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 8.x work is associated with a specific npm 8 release

Comments

@perrin4869
Copy link

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

Given the following:

❯ cat package.json
{
  "name": "npm854",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "install": "echo root install",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "workspaces": ["subpackage"]
}
❯ cat subpackage/package.json
{
  "name": "npm854-subpackage",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "install": "touch install",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}

Running npm install will not generate an install file inside the subpackage directory.

Expected Behavior

In 8.5.3 it would run the install script inside the workspaces as well
Probably introduced by #4529

Steps To Reproduce

No response

Environment

  • npm: 8.5.4
  • Node.js: 16.14.0
  • OS Name: slackware64-current
  • npm config:
; "user" config from /home/perrin4869/.npmrc

//npm.pkg.github.com/:_authToken = (protected)
//registry.npmjs.org/:_authToken = (protected)

; node bin location = /usr/bin/node
; cwd = /home/perrin4869/nodejs/npm854
; HOME = /home/perrin4869
; Run `npm config ls -l` to show all defaults.
@perrin4869 perrin4869 added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Mar 11, 2022
@wraithgar wraithgar added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Mar 15, 2022
unflxw added a commit to appsignal/appsignal-nodejs that referenced this issue Mar 28, 2022
@unflxw
Downgrade to NPM 8.5.3
9c3ece8 
In order to work around [a bug in NPM 8.5.4 and above][bug], where
lifecycle hooks are not executed for the workspace packages, use
NPM 8.5.3 for every Node version for now.

This is a redo of [#629][pr].

[bug]: npm/cli#4552
[pr]: #629
unflxw added a commit to appsignal/appsignal-nodejs that referenced this issue Mar 28, 2022
@unflxw
Downgrade to NPM 8.5.3
4f6f38f 
In order to work around [a bug in NPM 8.5.4 and above][bug], where
lifecycle hooks are not executed for the workspace packages, use
NPM 8.5.3 for every Node version for now.

This is a redo of [#629][pr].

[bug]: npm/cli#4552
[pr]: #629
@unflxw unflxw mentioned this issue Mar 28, 2022
Merged
@n8agrin
Copy link

n8agrin commented Mar 28, 2022

+1 I have a repro here: https://github.com/n8agrin/npm-8.5.4-no-lifecycle-scripts

It seems all lifecycle scripts in workspaces do not run in npm versions after 8.5.3

@n8agrin n8agrin mentioned this issue Mar 28, 2022
Merged
@koonpeng koonpeng mentioned this issue Mar 29, 2022
Closed
jeffkreeftmeijer pushed a commit to appsignal/appsignal-nodejs that referenced this issue Mar 31, 2022
@unflxw @jeffkreeftmeijer
Downgrade to NPM 8.5.3
092f8fa 
In order to work around [a bug in NPM 8.5.4 and above][bug], where
lifecycle hooks are not executed for the workspace packages, use
NPM 8.5.3 for every Node version for now.

This is a redo of [#629][pr].

[bug]: npm/cli#4552
[pr]: #629
@threema-danilo
Copy link

This also seems to happen if you don't use a workspace, but instead use a file:-dependency in package.json:

"eslint-plugin-xyz": "file:libs/eslint-plugin-xyz",

When running npm install in the main directory, with npm 8.5.0, the install script of the linked plugin is run, with npm 8.5.5 it is not run.

@ivancuric
Copy link

Issue is still present in npm 8.9.0.

@ruyadorno ruyadorno self-assigned this May 9, 2022
ruyadorno added a commit to ruyadorno/cli that referenced this issue May 10, 2022
@ruyadorno
fix(arborist): link deps lifecycle scripts
0d95e7b 
- Fixes running proper lifecycle scripts for linked deps and
  workspaces.
- Added test to validate lifecycle scripts don't run twice
  for linked deps
- Tweaked "reify workspaces bin files" test to also validate
  proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
  nodes to also validate that the install lifecycle scripts are
  also called.

Fixes: npm#4277
Fixes: npm#4552
Fixes: npm/statusboard#439
Relates to: npm#2905
ruyadorno added a commit to ruyadorno/cli that referenced this issue May 10, 2022
@ruyadorno
fix(arborist): link deps lifecycle scripts
0f8f932 
- Fixes running proper lifecycle scripts for linked deps and
  workspaces.
- Added test to validate lifecycle scripts don't run twice
  for linked deps
- Tweaked "reify workspaces bin files" test to also validate
  proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
  nodes to also validate that the install lifecycle scripts are
  also called.

Fixes: npm#4277
Fixes: npm#4552
Fixes: npm/statusboard#439
Relates to: npm#2905
@ruyadorno ruyadorno mentioned this issue May 10, 2022
Merged
wraithgar pushed a commit that referenced this issue May 10, 2022
@ruyadorno
fix(arborist): link deps lifecycle scripts (#4875)
5a50762 
- Fixes running proper lifecycle scripts for linked deps and
  workspaces.
- Added test to validate lifecycle scripts don't run twice
  for linked deps
- Tweaked "reify workspaces bin files" test to also validate
  proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
  nodes to also validate that the install lifecycle scripts are
  also called.

Fixes: #4277
Fixes: #4552
Fixes: npm/statusboard#439
Relates to: #2905
@ruyadorno
Copy link
Contributor

should be fixed in npm@8.10.0

unflxw added a commit to appsignal/test-setups that referenced this issue May 30, 2022
@unflxw
Pin broken builds to npm version 7
c16e518 
Using npm version 7 seems to avoid a bug where the build for the
`@appsignal/nodejs` package is not ran before `npm install`,
causing the installation to fail as it depends on the Transmitter,
which needs to be transpiled first.

The bug seems similar to [npm/cli#4552][bug], although that seems
to be fixed in npm 8.10.0, and upgrading to that version did not
fix the problem. I have been unable to reproduce the bug outside
of Semaphore CI, which further complicates investigating it.
@unflxw unflxw mentioned this issue May 30, 2022
Merged
shairyar pushed a commit to appsignal/test-setups that referenced this issue Aug 26, 2022
@unflxw
Pin broken builds to npm version 7
1c9a98b 
Using npm version 7 seems to avoid a bug where the build for the
`@appsignal/nodejs` package is not ran before `npm install`,
causing the installation to fail as it depends on the Transmitter,
which needs to be transpiled first.

The bug seems similar to [npm/cli#4552][bug], although that seems
to be fixed in npm 8.10.0, and upgrading to that version did not
fix the problem. I have been unable to reproduce the bug outside
of Semaphore CI, which further complicates investigating it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ruyadorno ruyadorno

Labels
Bug thing that needs fixing Priority 1 high priority issue Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(arborist): link deps lifecycle scripts ruyadorno/cli
6 participants
@n8agrin @wraithgar @ruyadorno @ivancuric @perrin4869 @threema-danilo