moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.05k stars 728 forks source link

Run buildAll in prepare, move husky install to postinstall #1583

Open schleyfox opened 5 months ago

schleyfox commented 5 months ago

This is part of a series of PRs based on performance work we have done to improve a use-case involving parsing/formatting hundreds of thousands of dates where luxon was the bottleneck.

This is just an improvement to make it possible to use luxon as a git dependency (for instance when you're testing out a bunch of hopefully good changes on your fork).

Husky adds git hooks into the repo. We want the dev experience of

$ git clone remote/luxon
$ npm install # installs husky hooks
$ ...develop develop develop...
$ git commit
_lint runs, everything works_

We also want to build luxon when we call npm pack or npm publish.

We also want to build luxon when it is installed somewhere else as a git dependency (https://docs.npmjs.com/cli/v10/configuring-npm/package-json#git-urls-as-dependencies)

Git dependencies work by running npm install from GitFetcher and then depend on DirFetcher running npm prepare (but only prepare, not prepack or postpack)

This change moves husky install to postinstall (which will, unfortunately, still pointlessly run when we install luxon as a git dependency). It moves the buildAll to prepare from prepack so that the build happens when installed as a git dependency. This should preserve existing behavior while enabling installation from git.

When installing from git and/or in CI environments, the husky command is sometimes not available in postinstall or is available but bails out when not run in the context of a git repo.

linux-foundation-easycla[bot] commented 5 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

schleyfox commented 5 months ago

/easycla

icambron commented 5 months ago

This change moves husky install to postinstall (which will, unfortunately, still pointlessly run when we install luxon as a git dependency).

I may just not understand how these things work, but I'd have guessed postinstall would run even when it's a regular, non-git npm dependency, which doesn't seem great.

schleyfox commented 5 months ago

yeah, that does seem possible, I'll test some more to see what's up. The docs around lifecycle scripts are confusing and, I strongly suspect, inaccurate (and the code isn't straightforward either). I'll test more and find a hook that works.

FWIW the docs

[npm install](https://docs.npmjs.com/cli/v9/commands/npm-install)
These also run when you run npm install -g <pkg-name>

preinstall
install
postinstall
prepublish
preprepare
prepare
postprepare
schleyfox commented 2 months ago

Finally got around to testing this, results in https://github.com/schleyfox/schleyfox-npm-lifecycle-test

npm install in the package dir (for e.g. development) runs

2024-05-03T15:11:39+0000 preinstall
2024-05-03T15:11:39+0000 install
2024-05-03T15:11:39+0000 postinstall
2024-05-03T15:11:39+0000 prepublish
2024-05-03T15:11:39+0000 preprepare
2024-05-03T15:11:39+0000 prepare
2024-05-03T15:11:39+0000 postprepare

while npm install in another app pulling the dependency from npm runs

2024-05-03T15:11:40+0000 preinstall
2024-05-03T15:11:40+0000 install
2024-05-03T15:11:40+0000 postinstall

install of a git dependency runs

2024-05-03T15:11:41+0000 preinstall
2024-05-03T15:11:41+0000 install
2024-05-03T15:11:41+0000 postinstall
2024-05-03T15:11:41+0000 prepublish
2024-05-03T15:11:41+0000 preprepare
2024-05-03T15:11:41+0000 prepare
2024-05-03T15:11:41+0000 postprepare
2024-05-03T15:11:41+0000 prepare
2024-05-03T15:11:41+0000 preinstall
2024-05-03T15:11:41+0000 install
2024-05-03T15:11:41+0000 postinstall

(I think that is an install (which includes prepare) followed by a bare prepare (no pre- or post-) followed by an npm-install-as-a-dependency (with pre- and post-)

npm publish runs

2024-05-03T10:26:20-0400 prepublishOnly
2024-05-03T10:26:20-0400 prepack
2024-05-03T10:26:20-0400 prepare
2024-05-03T10:26:20-0400 postpack
2024-05-03T10:26:50-0400 publish
2024-05-03T10:26:50-0400 postpublish

I think this indicates that prepare is the right place for husky to run, but it's also the right place to do the build. I'll update the prepare script to babel-node tasks/buildAll.js && (husky install || true)