[rush] circular dependence on postinstall

[WanderWang]

Summary

compiler
    |-- bin/my-cli.js
    |-- src/index.ts
    |-- dist/index.js  // git ignore and generated with rush build
    |-- tsconfig.json
project
    |-- package.json
rush.json // useWorkspaces=true ,  use pnpm

{ "postinstall":"my-cli"  }

on project's package.json

when I clone my repo and execute rush update ,it update both project and execute my-cli via postinstall,but execute fail because of compiler/dist/index.js should be generate after rush build

Repro steps

git clone https://github.com/egret-labs/egret/tree/rush-issue-2
cd egret
rush update

Expected result:

no exception

Actual result:

│ internal/modules/cjs/loader.js:883
│   throw err;
│   ^
│ Error: Cannot find module '../dist/index'

Details

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question	Answer
@microsoft/rush globally installed version?	5.35.2
rushVersion from rush.json?	5.35.2
useWorkspaces from rush.json?	true
Operating system?	windows
Would you consider contributing a PR?	no
Node.js version (node -v)?	14.15.4

[octogonz]

I confirmed that this only repros if useWorkspaces=true. It seems incorrect to be invoking postInstall scripts for a local workspace project that has not been built yet. (@zkochan is there any valid reason why PNPM has this behavior?)

Maybe this can be fixed by appending --ignore-scripts to the pnpm install command line (during rush install). @D4N14L @ThomasMichon do you see any problem with that?

[zkochan]

This is the default behavior of all package managers. postinstall is executed after installation.

[octogonz]

@zkochan But for a local workspace project whose source code has not been compiled yet, would you consider that to be "installed"? Does this mean that postinstall scripts must never reference a built output?

[zkochan]

Seems like this is the same issue as pnpm/pnpm#1801

[octogonz]

Thinking about this more, I'm fairly certain that PNPM's behavior here is not right. (We can use pnpm/pnpm#1801 to discuss that.)

It seems that we can't use --ignore-scripts as a workaround because it applies to all dependencies, not just local projects. Maybe we can use pnpmfile.js to disable the scripts selectively? I can't think of any other workarounds.

[octogonz]

@zkochan looking at pnpm/pnpm#1801 more closely, it involves the prepare lifecycle event, not postInstall. So it seems to be not the same issue.

In my opinion PNPM's handling of postInstall here is plainly incorrect:

• postinstall scripts are meant to be executed after the package manager installs a package
• Whereas for local projects that have not been built yet, arguably PNPM is linking them, not installing (at least from an end user's perspective). For example, npm link performs analogous behavior, but it does not invoke this script.
• In any case, PNPM's behavior implies that a postinstall script must never refer to any build output, which is definitely NOT how people use these scripts in practice
• I cannot think of any real world cases where it would actually be desirable to invoke a postInstall action of a project that has not been compiled yet
• But if there was such a case, the action could be performed by the build step. (Or PNPM could provide a separate postLink step for workspaces.)

[zkochan]

This is how the npm CLI works.

[zkochan]

The linked issue will solve this issue if the @egret/link-node-modules package will have a "prepare": "pnpm build" script

[octogonz]

NPM does not support workspaces.

[zkochan]

Actually, it does since v7 https://docs.npmjs.com/cli/v7/using-npm/workspaces

and it behaves the same when inside a workspace

[octogonz]

I see. Hmm. It does make sense for PNPM to behave consistently with NPM. But also the design seems wrong. So maybe we could provide a setting to fix this in Rush repos. And maybe the setting would be enabled by default.