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

Prevent automatic global installation of wasm-pack #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ThisIsMissEm
Copy link

Installing software globally on a developers' machine as a side-effect of using a webpack plugin is generally a bad idea, especially when they are not informed that this will take place. Instead, it's better to provide instructions as to how someone can install the relevant software, and allow a user to choose.

Automatic installation can also be problematic in certain CI environments where there are stricter security requirements.

This automatic installation was added in: aea762a

Installing software globally on a developers' machine as a side-effect of using a webpack plugin is generally a bad idea, especially when they are not informed that this will take place. Instead, it's better to provide instructions as to how someone can install the relevant software, and allow a user to choose.

Automatic installation can also be problematic in certain CI environments where there are stricter security requirements.
@wleslie
Copy link
Contributor

wleslie commented Sep 12, 2021

Actually I'm unsure why wasm-pack-plugin can't just declare an ordinary dependency on wasm-pack in its package.json file, instead of either (a) downloading/installing it automatically, or (b) making the user do it manually.

@ThisIsMissEm
Copy link
Author

ThisIsMissEm commented Sep 12, 2021 via email

@woss
Copy link
Contributor

woss commented Sep 29, 2021

it's cool to have the wasm-pack on the npm, I vote against globally installing it especially inside the plugin itself.

I see the following options:

  1. fail if the wasm-pack is not found, it's what this PR does
  2. declare the wasm-pack as a dependency in the package.json

CASE 1: user decides will they install it via cargo or npm
CASE 2: user has less control over how the wasm-pack is installed. at least is not installed globally

Given that this plugin is part of the nodejs ecosystem, CASE 2 makes a lot more sense.

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.

3 participants