wireit doesn't work with preinstall scripts

[RJWadley]

I'd like to use wireit for preinstall scripts:

"preinstall": "wireit",
"pnpm:devPreinstall": "wireit",

Obviously, what you can do in a preinstall script is fairly limited. packages won't be available, but you can still do scripting related stuff.

Using the usual configuration here will fail because wireit has not yet been installed:

. pnpm:devPreinstall$ wireit
│ sh: wireit: command not found
└─ Running...
 ELIFECYCLE  Command failed.
. preinstall$ wireit
│ sh: wireit: command not found
└─ Running...
 ELIFECYCLE  Command failed.

The obvious fix is to use pnpx or similar to ensure the package is downloaded:

"preinstall": "pnpx wireit",
"pnpm:devPreinstall": "pnpx wireit",

But wireit itself complains in that scenario:

. pnpm:devPreinstall$ pnpx wireit
│ ❌ package.json:16:25 This command should just be "wireit", as this script is configured in the wireit section.
│     ning...   "pnpm:devPreinstall": "pnpx wireit",
│                             ~~~~~~~~~~~~~
│     package.json:124:3 The wireit config is here.
│         ...           "pnpm:devPreinstall": {
│           ~~~~~~~~~~~~~~~~~~~~
└─ Failed in 404ms at /Users/robbie/programming/work/gen-1
 ELIFECYCLE  Command failed with exit code 1.

I'd suggest relaxing wireit's check here to allow invoking wireit via a package manager within it's script, eg:

npx wireit
bunx wireit
pnpx wireit
pnpm dlx wireit

[bitmage]

I've implemented my "Third Option" as mentioned in the link above:

main...TorchlightSoftware:wireit:main

I'm not sure why it's so important that the wireit command is so constrained? I think perhaps the authors are trying to eliminate or reduce the chance that people accidentally use the wireit command to create recursive loops?

I don't know if you'd all be comfortable with a relaxed constraint like this, or if not maybe an "--i-know-what-im-doing" or similar.

In any case I've tested this with my code base, and this simple change does allow me to receive environment variables into a whole command chain and act on them. I'm willing to submit a pull request, but I think it requires more discussion to be sure the implementation is in alignment with the project's design goals.

[aomarks]

I've implemented my "Third Option" as mentioned in the link above:

Thank you for the PR!

I'm not sure why it's so important that the wireit command is so constrained? I think perhaps the authors are trying to eliminate or reduce the chance that people accidentally use the wireit command to create recursive loops?

The main reason we constrain the wireit command is to avoid the confusing situation of having an npm script that behaves one way when invoked directly, but another way when invoked as a dependency of another wireit script.

One way this could happen is the upgrade case: Imagine I have an existing npm script. I decide to migrate it to wireit. I configure the "wireit" section. I forget to change the original script command to wireit. Now running the build script directly not through wireit is likely to do something unintended that breaks my build.

Additionally, there is the question of additional arguments. If we lift the restriction, the current behavior for npm (I still need to check pnpm and yarn) would be to treat an npm script configured like {"build": "wireit --watch"} as equivalent to npm run build -- --watch. I think that's a bit confusing, since wireit --watch seems like it should affect wireit's behavior, but actually it will affect the underlying script.

While investigating this, I also noticed a new warning from npm which means the npm run build --watch style will break in v12 because of some new tightening for npm flag handling. I just filed #1346 to deal with that, thoughts welcome.

I don't know if you'd all be comfortable with a relaxed constraint like this, or if not maybe an "--i-know-what-im-doing" or similar.

Yeah, I think that's a good idea. How about we relax the validation to simply require that the string "wireit" appears anywhere in the command? This allows all of the forms to be used (including the escape hatch of appending a #wireit comment to any arbitrary command), but I think still provides sufficient protection for the main concern. Feel free to update your PR to do this.

I do want to figure out the plan for the related flag handling issue before releasing that change, though.

[bitmage]

Well, and now thank you for the timely and thoughtful response! Very much appreciated.

Additionally, there is the question of additional arguments. If we lift the restriction, the current behavior for npm (I still need to check pnpm and yarn) would be to treat an npm script configured like {"build": "wireit --watch"} as equivalent to npm run build -- --watch. I think that's a bit confusing, since wireit --watch seems like it should affect wireit's behavior, but actually it will affect the underlying script.

That's insightful, and not something I anticipated.

Yeah, I think that's a good idea. How about we relax the validation to simply require that the string "wireit" appears anywhere in the command?

That's the change I implemented in my fork.

I do want to figure out the plan for the related flag handling issue before releasing that change, though.

Understood. You probably don't need a pull request from me since this is a one liner. And for now my fork gives me a workaround that gets my own codebase functioning. I've published my fork at @tlsft/wireit in case anyone else wants to use it. No promises I will keep this up to date with upstream changes - I hope my maintenance of the fork will be short lived. My fork also contains an implementation of "sequence" which we should talk about further.