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

Install npm dependencies automatically when creating apps. #8108

Merged
merged 1 commit into from Nov 28, 2016

Conversation

Projects
None yet
2 participants
@benjamn
Member

benjamn commented Nov 28, 2016

Although I have said I do not think Meteor should run npm install automatically to update application node_modules, my real concern is that Meteor should not interfere with your preferred node_modules-related workflow, be it npm, npm-shinkwrap.json, shrinkpack, yarn + yarn.lock, checking your node_modules into git/mercurial/cvs, or whatever other scheme you currently have (or might adopt in the future).

Automatically installing node_modules from the default package.json file when a new app is created will eliminate real confusion, and should not interfere with any workflows, because there is no opportunity to establish another workflow before Meteor runs npm install the very first time.

Addresses #6848.
May make #8095 unnecessary?

Install npm dependencies automatically when creating apps.
Although I have said I do not think Meteor should run `npm install`
automatically to update application node_modules, my real concern is that
Meteor should not interfere with your preferred node_modules-related
workflow, be it npm, npm-shinkwrap.json, yarn, yarn.lock, checking your
node_modules into git/mercurial/cvs, or whatever other scheme you have.

Automatically installing node_modules from the default package.json file
when a new app is created will eliminate real confusion, and should not
interfere with any workflows, because there is no opportunity to establish
another workflow before Meteor runs `npm install` the very first time.

@benjamn benjamn added this to the Release 1.4.3 milestone Nov 28, 2016

@benjamn benjamn merged commit ac86594 into devel Nov 28, 2016

4 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@glasser

This comment has been minimized.

Member

glasser commented Nov 29, 2016

Revert 41713da now?

benjamn added a commit that referenced this pull request Feb 12, 2017

abernix added a commit that referenced this pull request Feb 13, 2017

benjamn added a commit that referenced this pull request Feb 13, 2017

glasser added a commit that referenced this pull request Feb 27, 2017

Remove `meteor npm install` instruction
As of #8108, `meteor create` runs `meteor npm install`, so it doesn't need to tell you do to do so.

abernix added a commit that referenced this pull request Feb 28, 2017

Remove `meteor npm install` instruction (#8433)
As of #8108, `meteor create` runs `meteor npm install`, so it doesn't need to tell you do to do so.

abernix added a commit that referenced this pull request Mar 1, 2017

Remove `meteor npm install` instruction (#8433)
As of #8108, `meteor create` runs `meteor npm install`, so it doesn't need to tell you do to do so.

@abernix abernix referenced this pull request Mar 16, 2017

Merged

Removed NPM Install #89

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