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] npm i in workspace runs prepare script concurrently #4100

Open
2 tasks done
fishcharlie opened this issue Nov 29, 2021 · 9 comments
Open
2 tasks done

[BUG] npm i in workspace runs prepare script concurrently #4100

fishcharlie opened this issue Nov 29, 2021 · 9 comments
Labels
Documentation documentation related issue Enhancement new feature or improvement Priority 2 secondary priority issue Release 8.x work is associated with a specific npm 8 release

Comments

@fishcharlie
Copy link

fishcharlie commented Nov 29, 2021

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 i runs Prepare script concurrently across all workspaces.

This causes major problems when using TypeScript and having one workspace dependent on another, since the build (prepare script) runs concurrently, there is no way to ensure the prepare command is done on one workspace before running on the next.

Expected Behavior

It to actually follow what the documentation says (https://docs.npmjs.com/cli/v7/using-npm/workspaces#running-commands-in-the-context-of-workspaces):

Screen Shot 2021-11-28 at 5 01 30 PM

Nothing in there says anything about commands being run concurrently, or at the same time.

Steps To Reproduce

  1. Create 2 workspaces
  2. Add a prepare command to each where the second is reliant on the first completing
  3. Run npm i
  4. Notice that everything fails since the commands are run concurrently

Environment

  • npm: 8.1.0
  • Node: v16.13.0
  • OS: macOS 12.1 Beta (21C5039b)
  • platform: MacBook Pro M1 Max
  • npm config:
; "user" config from /Users/charliefish/.npmrc

//npm.pkg.github.com/:_authToken = (protected)
//registry.npmjs.com/:_authToken = (protected)
registry = "https://npm.network.charlie.fish/"

; node bin location = /Users/charliefish/.nvm/versions/node/v16.13.0/bin/node
; cwd = /Users/charliefish
; HOME = /Users/charliefish
; Run `npm config ls -l` to show all defaults.
@fishcharlie
Copy link
Author

If anyone is able to provide a workaround to this I'd really appreciate it.

@fishcharlie
Copy link
Author

It's been over a week since I've posted this. Can I please get a reply here? This is a major blocker for me.

@fishcharlie
Copy link
Author

@ljharb Sorry to tag you randomly here. I notice you've been pretty active in replying to other issues.

Is there any chance you could point me in the right direction here? Who on the npm team would be best to assist here?

This is a major blocker for my project currently.

Thanks in advance!!

@ljharb
Copy link
Collaborator

ljharb commented Dec 13, 2021

I'm not sure who's owning workspaces atm, sorry.

@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 13, 2021

Hey @fishcharlie thanks for your patience. Had to coordinate between a couple people being out of office to get an answer

The docs you linked are referring to running commands in workspaces, not the internal lifecycle scripts that happen during reification. So for instance if you were to run npm run somecommand --ws then somecommand would be ran in every workspace in the order they appear in your package.json.

As far as deterministically ordering prepare scripts, this is not possible. Here is a comment from a previous issue discussion why it can not happen in the current npm version #3034

@fishcharlie
Copy link
Author

@MylesBorins Thanks for the reply!

The docs you linked are referring to running commands in workspaces, not the internal lifecycle scripts that happen during reification

Got it. I think the documentation can be improved in this regard to make that more clear.

As far as deterministically ordering prepare scripts, this is not possible

I guess this leads to a question about how I should be doing this instead? I don't think my use case and what I'm trying to achieve is that out there (with having a workspace, with a primary project and dependencies, all in TypeScript).

If you look at the npm documentation for prepare, once of the use cases listed is: Compiling CoffeeScript source code into JavaScript. Arguably compiling TypeScript code into JS is the same concept. So based on that, doing my tsc build step within the prepare command is correct.

However, because my primary package in my workspace has a dependency on the build step from the utilities package being complete first, that leads to the issues mentioned here.

So is there something I should change in my workspace settings to do this correctly?

Here is a comment from a previous issue discussion why it can not happen in the current npm version

Although I understand the deadlocking situation presented in #3034, it's just as valid to not have circular dependencies, and require the prepare script to be run sequentially.

It sounds like a workaround would be to use --foreground-scripts maybe?? But there is no way within the package.json to set that option as the default, and as a result, contributors to the open source project will get confused when trying to run npm install with a bunch of errors. So I'm not sure that is a realistic workaround.


Although everything you said makes sense, I'm still super confused about how to handle this for dynamoose.

I understand that this is getting out of the scope of "it's a bug" and getting into more of "how should I handle this", which might be out of scope for GitHub Issues. But I do think my situation is a common/valid use case that npm needs to support or consider.

@fishcharlie
Copy link
Author

@MylesBorins Following up on this. Is there anything I can clarify in my previous message?

@lukekarrys lukekarrys removed Needs Triage needs review for next steps Bug thing that needs fixing labels Feb 27, 2022
@lukekarrys
Copy link
Contributor

But there is no way within the package.json to set that option as the default

Running npm config set foreground-scripts=true --location=project should work to set that as a project level config in an .npmrc file. If you commit this file, it will be applied to all commands run in the project for contributors.

I'm removing the bug label but keeping this open with the following action items:

  • Documenting that prepare scripts run concurrently
  • Add an option to run them sequentially without needing to turn on --foreground-scripts

@lukekarrys lukekarrys added Documentation documentation related issue Enhancement new feature or improvement labels Feb 27, 2022
@lukekarrys lukekarrys added the Priority 2 secondary priority issue label Apr 10, 2022
@marijnh
Copy link

marijnh commented May 3, 2023

As far as deterministically ordering prepare scripts, this is not possible.

Are you clear on how big of a damper this is on the usefulness of workspaces? In TypeScript setups, or sets of modules that use bin commands from each other, etc, there doesn't seem to be a way to reliably install the workspace, at all.

--foreground-scripts doesn't appear to provide control over the order in which install scripts are run either. The order in the workspaces field doesn't seem to be taken into account there. So even in a completely trivial setup like below, I have not found a way to force b's install script to run before a's.

package.json:

{"workspaces": ["b", "a"]}

a/package.json:

{
  "name": "test-a",
  "version": "1.0.0",
  "scripts": {
    "prepare": "echo prep A"
  }
}

b/package.json:

{
  "name": "test-b",
  "version": "1.0.0",
  "scripts": {
    "prepare": "echo prep B"
  }
}

If I run npm i --foreground-scripts (with npm 9.6.5), A runs before B.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation documentation related issue Enhancement new feature or improvement Priority 2 secondary priority issue Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

No branches or pull requests

5 participants