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

Use existing NPM package manager when installing Breeze dependencies #188

Closed
wants to merge 4 commits into from
Closed

Use existing NPM package manager when installing Breeze dependencies #188

wants to merge 4 commits into from

Conversation

daleweaver777
Copy link
Contributor

PR #180 added functionality to automatically install NPM packages and builds your assets. However, it defaults to using npm instead of the package manager you are currently using in your project (like pnpm or yarn).

This PR adds functionality to check if you are already using a package manager for your project and uses that to install Breeze dependencies. If you aren't using a package manager, it defaults to npm.

@daleweaver777
Copy link
Contributor Author

I use pnpm for my package manager. Much better/faster than npm.

Since Breeze and Jetstream installers automatically run npm install, I need to delete node_modules and package-lock.json on every new project and then run pnpm install.

If I ran pnpm install before running the Breeze or Jetstream installers, I get npm errors because it's trying to using npm to install new dependencies.

One option would be to prompt the user during the Breeze and Jetstream installation to specify the npm package manager they want to use.

Another option would be to pass a flag to the installer if you don't want to run npm install.

The third option is this PR. First check if a npm package manager was already used in the project. If so, use it or default to npm.

@jessarcher
Copy link
Member

Hey, thanks for the PR!

I'm not opposed to using the users' preferred package manager, however, Breeze is only intended to be installed on fresh Laravel applications where there shouldn't be a lock file. I worry that this behaviour implicitly adds support or expectations for Breeze to work with existing applications when there are just too many variables to support it properly.

Additionally, if the user installs Breeze via Sail, the presence of one of these lock files doesn't guarantee that the command will be present in that environment.

@daleweaver777
Copy link
Contributor Author

Totally understand that you don't want to encourage users to install Breeze or Jetstream into existing projects. I would be happy with simply passing a flag to specify package manager, or a flag to not run npm at all.

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.

2 participants