Publishing with npm@7 publishes incorrect build of compiler

[Conduitry]

Running npm publish with npm@7 produces the proper PUBLISH=true build of the compiler, runs tests against it, and then apparently re-creates a non-PUBLISH=true build right before packaging things up to publish them. I tried fixing this in e2fa0e0 but that broke Windows builds and I reverted it.

I tried looking at npm@7's changelog to see what changed that would have caused this, but didn't find anything. I don't see any references to the prepare script being re-run. For now, I'm just going to publish with npm@6, but we should sort this out sometime soon.

[Conduitry]

Now that npm@latest is v7, I took another look at this, and it appears to still be happening. I'm really tempted to just remove the prepare script, but I'm worried about that breaking stuff for people who are installing from Git branches or forks and rely on install-time building of the compiler.

[Conduitry]

I did just try that, and removing the prepare script seemed to make this work with npm v7. I still don't get why this change is happening though. I don't see any relevant differences between https://docs.npmjs.com/cli/v6/using-npm/scripts and https://docs.npmjs.com/cli/v7/using-npm/scripts.

[Conduitry]

npm seems to be trying really hard to hide when and how the build script is getting run the second time. I put lots of && echo &&s and && env &&s into various scripts, but I never saw any logs triggered by the second build. (I guess if they succeeded, npm swallowed stdout?) I eventually caught it by doing sudo chown root compiler.js while the tests were running - and now I finally have some error logs that I haven't been able to understand yet.

[Conduitry]

Okay, it's not just an environment variable difference between v6 and v7. With the same chown trick, npm publlish finishes fine in v6, meaning the build script is never even called again.

[Conduitry]

The difference is the order that prepare and prepublishOnly are called. I've opened npm/cli#2668 to find out whether this is a bug.

[Conduitry]

We shouldn't just remove the prepare script, I'm realizing, since we depend on that to produce the initial build required for linting the project. This would (among other things) break CI.

[Conduitry]

Updating the lifecycle docs to reflect reality is a TODO on npm's end - npm/statusboard#267

According to the WIP PR npm/cli#2690 it sounds like it is expected that prepare run after prepublishOnly, which means that we'll need some more permanent way of handling this. I don't know whether #5983 is that solution. But it should at least prevent bad builds from being published, so I'm going to merge it now, but keep this issue open.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Conduitry]

I'm not sure what labels mean what now, but I think this is still relevant. Publishing is still inelegant, to work around what is apparently the new intended way for lifecycle scripts to work.

[benmccann]

I didn't look at this closely, but it sounds similar to an issue I hit in the sites repo. You can take a look at my solution there to see if it'd help: sveltejs/sites#53

[gtm-nayan]

Closing as no longer relevant