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

Thoughts about version 3.2.0 #726

Closed
oles opened this issue Jan 3, 2019 · 18 comments
Closed

Thoughts about version 3.2.0 #726

oles opened this issue Jan 3, 2019 · 18 comments

Comments

@oles
Copy link

oles commented Jan 3, 2019

Two things I noticed when I ran npm install when updating webpack-cli to version 3.2.0:

screenshot from 2019-01-03 15-07-20

There's now a rather large Open Collective banner being outputted, and 49 new dependencies.

I'm fine with the banner, but a little put off by all those dependencies - seeing I only need it to be able to run webpack to build my project.

Seeing that people reacted to a much less intrusive banner for nodemon (remy/nodemon#1189), perhaps a solution similar to the one @remy merged would be suitable?

@evenstensberg
Copy link
Member

Yes, the logo should be removed. For the dependencies, a dep for recursively finding a config was used. Sorry if this causes any trouble. I’ll add a util function for it instead in a week or so after grouping up potential issues.

@oles
Copy link
Author

oles commented Jan 3, 2019

I see - thanks for sharing!

@evenstensberg
Copy link
Member

Keeping this open as it's a enhancement issue

@evenstensberg evenstensberg reopened this Jan 4, 2019
@edmorley
Copy link

edmorley commented Jan 4, 2019

It's worth noting that the use of a postinstall script entry (or any of the equivalents, such as preinstall) means that Yarn's new plug and play feature has to unpack the package (in order to support packages that create additional files in the package directory as part of installation) negating some of the benefits of the plug and play mode.

A workaround for this (which also stops the console spam) is to use Yarn's --ignore-scripts option (either on the CLI or via a .yarnrc in the repo root), which stops postinstall et al running entirely, and is good to use for security reasons regardless (see here).

@AviVahl
Copy link
Contributor

AviVahl commented Jan 5, 2019

the new deps are due to webpack-cli@3.2.0 installing opencollective, and its outdated dependencies:

opencollective@^1.0.3:
  version "1.0.3"
  resolved "https://registry.yarnpkg.com/opencollective/-/opencollective-1.0.3.tgz#aee6372bc28144583690c3ca8daecfc120dd0ef1"
  integrity sha1-ruY3K8KBRFg2kMPKja7PwSDdDvE=
  dependencies:
    babel-polyfill "6.23.0"
    chalk "1.1.3"
    inquirer "3.0.6"
    minimist "1.2.0"
    node-fetch "1.6.3"
    opn "4.0.2"

👎

One of the improvements in v3 was less unneeded deps, and this change doesn't follow suit.

@evenstensberg
Copy link
Member

OpenCollective banner stays for reachability. Making logo a bit smaller, maybe I'll write a script for it and remove the dep, who knows :)

@evenstensberg
Copy link
Member

npm install --save webpack-cli version 3.2.1

@bjornstar
Copy link

Some user feedback here, I'm not going to update my instances of webpack-cli with your postinstall advertising in it. It's very unwelcome.

@evenstensberg
Copy link
Member

Feel free to use another alternative, nothing stopping you :)

@bjornstar
Copy link

Thanks, your response motivated me to remove webpack-cli from my project.

Since all I was using it for was build this is what I switched to:

const webpack = require("webpack");

const pkg = require("../package.json");
const config = require("../webpack.config.js");

config.mode = "production";

const compiler = webpack(config);

console.log(`Building ${pkg.name} v${pkg.version}`);

compiler.run((error, stats) => {
  if (error) {
    console.error(error);
    return process.exit(1);
  }
  console.log(`Built in ${stats.endTime - stats.startTime}ms`);
});

@AviVahl
Copy link
Contributor

AviVahl commented Jan 17, 2019

@bjornstar above script is catching webpack process errors, but not compilation ones. See https://webpack.js.org/api/node/#error-handling

@edmorley
Copy link

An alternative is to install all dependencies using the --ignore-scripts option. This means yarn/npm do not invoke the preinstall/install/postinstall lifecycle scripts at all, which in most cases does not cause breakage since they are for optional steps. This also improves security slightly, since it stops the execution of arbitrary code at package installation time.

To enforce this for a project when using Yarn, add --ignore-scripts true to a .yarnrc in the same directory as the package.json and commit it to the repository. (I'm presuming something similar works for npm via .npmrc)

@AviVahl
Copy link
Contributor

AviVahl commented Jan 17, 2019

--ignore-scripts does break things for many projects out there.

puppeteer, for example, uses install to download a compatible Chrome version. node-sass uses postinstall to download pre-compiled Node addon, or to build one using node-gyp.

Using lifecycle scripts to communicate information to your users (when they install the package) feels like an abuse.

@ematipico
Copy link
Contributor

I don't understand. The last version doesn't have the banner. What's the real deal breaking? Please help me to understand

@evenstensberg
Copy link
Member

They’re not happy to have a banner asking for financial help for a software they’re using for free and take for granted :))

@AviVahl
Copy link
Contributor

AviVahl commented Jan 17, 2019

I don't understand. The last version doesn't have the banner. What's the real deal breaking? Please help me to understand

There's still a banner.

avi@localhost  ~/projects/cli-banner  npm i webpack-cli -D

> webpack-cli@3.2.1 postinstall /home/avi/projects/cli-banner/node_modules/webpack-cli
> lightercollective


     *** Thank you for using webpack-cli! ***

Please consider donating to our open collective
     to help us maintain this package.

  https://opencollective.com/webpack/donate

                    ***

They’re not happy to have a banner asking for financial help for a software they’re using for free and take for granted :))

It's not the banner itself that bothers me. It's the place where it was integrated to. Communication to users should not be done via package lifecycle scripts IMO.

@edmorley
Copy link

edmorley commented Jan 17, 2019

It's not the banner itself that bothers me. It's the place where it was integrated to. Communication to users should not be done via package lifecycle scripts IMO.

Agreed. Perhaps the CLI could include a small prompt on second or third run, that's only shown once every week/month/... or something?

They’re not happy to have a banner asking for financial help for a software they’re using for free and take for granted :))

I agree that there are a significant number of users who hold this mentality with open source in general, but as someone who contributes to the webpack ecosystem (amongst others) with code contributions (and happily no compensation), I find this sweeping generalisation a little frustrating.

@evenstensberg
Copy link
Member

I will change this in a newer release. Sorry for the inconveniences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants