Post-install script causes failures

[moritonal]

Describe the Bug When installing the lib on Windows it errors with the line npm ERR! The token '||' is not a valid statement separator in this version..

To Reproduce Install the library.

Expected Behavior To install correctly.

Cause The issue is that you have a hacky post-install step in the package.json which errors depending on the scripting environment of the installer.

Logs

PS C:\...> npm i
npm ERR! code 1
npm ERR! path C:\...\node_modules\type-graphql
npm ERR! command failed
npm ERR! command powershell -c node ./dist/postinstall || exit 0
npm ERR! At line:1 char:25
npm ERR! + node ./dist/postinstall || exit 0
npm ERR! +                         ~~
npm ERR! The token '||' is not a valid statement separator in this version.
npm ERR!     + CategoryInfo          : ParserError: (:) [], ParentContainsErrorRecordException
npm ERR!     + FullyQualifiedErrorId : InvalidEndOfLine

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\tmp\npm-cache\_logs\2022-08-28T22_11_38_524Z-debug-0.log

Environment (please complete the following information):

OS: Windows Node: v16.15.0

Name                           Value
----                           -----
PSVersion                      5.1.22000.653
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.22000.653
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Additional Context This is a pretty clear-cut case of postinstall abuse, but I can work around it with the --ignore-scripts flag or a fork. I'd recommend you look at using the npm fund process now.

[MichalLytek]

I've been using && and || in npm script for ages. Maybe because I had npm configured to run scripts by cmd, not powershell. TBH, it's weird that you're the first one that complains about it 🤔

I wonder if we can just remove the || exit 0 for now, and then work on changing the Open Collective info displayed on install. WDYT?

[carlocorradini]

@moritonal I've never had this issue. Nevertheless, I suggest you to update PowerShell to a newer version. See https://github.com/PowerShell/PowerShell for more information.

[moritonal]

Yes, you are correct that Powershell 5.1.22000.832 throws an error when running echo "test" || exit 0 whilst Powershell 7.2.6 does not, so the problem is likely to lesson over time.

My advice is simply to remove the postinstall script, it's against the OpenCollectives recommendation, leaks data into the host's machine via configstore, and likely has failed on other peoples machines and they simply didn't say anything thinking the package itself is broken.

However, I do appreciate it is a revenue stream for you, so of course it's your choice how you handle this.

[carlocorradini]

I was wondering if we can move postinstall.ts from src to bin and make it a simple JavaScript (.js) file as Nodemon does.

[MichalLytek]

@carlocorradini What is the benefit of such change? Now we run .js from dist so it throws error only on development of this package... so thanks to || exit 0 it does not break the npm install.

I think I would switch to npm fund anyway, can someone from community list me some examples of graphql/node popular libs/framework and their approach for promoting their OpenCollective fund?

[carlocorradini]

See this from Open Collective. Simply removes the postinstall script. 🥳

[carlocorradini]

@MichalLytek I can create a PR to solve this issue. What do you prefer:

1. funding in package.json only
2. Installation script in pure JS (Nodemon style) in a dedicated bin directory?

[MichalLytek]

Let's do 1., should be no conflict with the v2.0 branch