Lychee
Lychee copied to clipboard
LycheeOrg
Run `npm install` in post-merge hook
To update JS dependencies just like Composer dependencies. Only runs in dev mode.
But don't we include the compiled JS file already in the Git repo?
But then what is the point of npm install ? :) I am not sure how this is actually done in this case, but I don't think the npm install is necessary. I may be wrong.
It updates the JS dependencies in node_modules that will then be used if you call mix. Similar to what composer install is doing with PHP dependencies. If the dependencies aren't installed, you can't use mix.
It updates the JS dependencies in
node_modulesthat will then be used if you call mix. Similar to whatcomposer installis doing with PHP dependencies. If the dependencies aren't installed, you can't use mix.
In that case, I'd suggest both or neither. I'd lean towards neither, especially as we're not really using livewire yet.
The more I think of it, the more unsure I am as we commit the JS and css files. As a result we do not need to compile them each times. Otherwise this requires an additional nodeJs dependency which I am not too happy to add for users. I would rather have php being the only requirement. :)
The more I think of it, the more unsure I am as we commit the JS and css files. As a result we do not need to compile them each times. Otherwise this requires an additional nodeJs dependency which I am not too happy to add for users. I would rather have php being the only requirement. :)
Total agreement. I don't like adding development dependencies to the main line. But it is up to you, @ildyria, you approved this PR. If you are getting second thoughts, then you should revoke the approval. I am not a big fan of this PR.
I have already asked in https://github.com/LycheeOrg/Lychee/pull/1440#issuecomment-1207189357 why we need NPM at all, because I have never needed it before.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}