Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.x] Install NPM dependencies and build assets #1119

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Aug 10, 2022

This PR updates the Jetstream installer to automatically run the npm install and npm run build commands for you.

Currently, the Laravel installer automatically runs these commands when it installs Jetstream. Moving it to this package makes the installation more consistent no matter how you install it. I have created laravel/installer#242 to remove these from the Laravel installer.

  • Tested on Linux
  • Tested on Mac
  • Tested on Windows

@joelbutcher
Copy link
Contributor

Love this! I'm currently doing this for users of Socialstream 🎉

@joelbutcher
Copy link
Contributor

@jessarcher ohh just had a thought - with this addition to Jetstream, users of my package would now need to wait twice whilst the frontend is compiled... what are your thoughts on making this option configurable for other package maintainers that rely on Jetstream as a basis for their packages?

@timacdonald
Copy link
Member

I have just tested this on my Mac, and the NPM install process worked as expected and had the colourised output as well 🤩

@jessarcher
Copy link
Member Author

Hey @joelbutcher, how much of an impact do you expect this to have?

I don't see an issue with npm install running twice, as the second time it should be very quick to realise nothing needs to be done. You could probably even remove this from your package if you aren't changing package.json at all.

As for the build, it's a bit unfortunate that it would run twice, but the build is very fast with Vite.

I'm hesitant to add another option to the install command, but if this PR is accepted, you're of course welcome to open a PR for it.

@jessarcher jessarcher marked this pull request as ready for review August 12, 2022 06:32
@joelbutcher
Copy link
Contributor

@jessarcher The impact probably wouldn't matter that much. I'm not updating the package.json AFAIK (will check later). The build time with Vite is much less of a concern actually, I hadn't considered that tbh

@taylorotwell taylorotwell merged commit 28968ff into 2.x Aug 12, 2022
@taylorotwell taylorotwell deleted the npm-install-and-build branch August 12, 2022 17:15
sinan-aydogan added a commit to sinan-aydogan/tailadmin-core that referenced this pull request Aug 13, 2022
[2.x] Install NPM dependencies and build assets (laravel#1119)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants