FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Remove bundledDependencies by fson · Pull Request #1068 · react/create-react-app · GitHub

/ create-react-app Public

Remove bundledDependencies - #1068

Merged
fson merged 1 commit into
react:masterfrom
fson:remove-bundled-deps
Nov 21, 2016
ConversationCommits (1)ChecksFiles changed
Merged

Remove bundledDependencies#1068
fson merged 1 commit into
react:masterfrom
fson:remove-bundled-deps

Conversation

fson commented Nov 20, 2016

Copy link
Copy Markdown
Contributor
  • Remove bundledDependencies
  • Change the e2e scripts to use local file dependencies instead of bundledDependencies to test the packages

Remove bundledDependencies
e5ab354 
* Remove bundledDependencies
* Change the e2e scripts to use local file dependencies instead of
  bundledDependencies to test the packages
gaearon reviewed Nov 20, 2016
View reviewed changes
Comment thread tasks/release.sh

# This modifies package.json to copy all dependencies to bundledDependencies
node ./node_modules/.bin/bundle-deps

gaearon Nov 20, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good riddance

camwest Nov 22, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you explain why bundle-deps was removed?

gaearon Nov 22, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Because of many issues with them due to npm's buggy client implementation.

fson Nov 22, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Curiously, the bundled dependencies weren't really speeding up the installation either. I did a quick benchmark comparing the latest nightly build (without bundled deps) and 0.7.0 and there was no discernible difference in the time to create a new app. With a warmed up cache, the new version without bundled deps was actually outperforming the old version that had them.

gaearon Nov 23, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe it was only true for earlier versions with a different dependency structure or something like this.

gaearon commented Nov 20, 2016

Copy link
Copy Markdown
Contributor

If Travis passes lgtm.

gaearon added this to the 0.8.0 milestone Nov 20, 2016
thien-do mentioned this pull request Nov 21, 2016
Closed
fson merged commit e042634 into react:master Nov 21, 2016
fson deleted the remove-bundled-deps branch November 21, 2016 11:26
fson mentioned this pull request Nov 22, 2016
Closed
jarlef pushed a commit to jarlef/create-react-app that referenced this pull request Nov 28, 2016
Remove bundledDependencies (react#1068)
27d6e93 
* Remove bundledDependencies
* Change the e2e scripts to use local file dependencies instead of
  bundledDependencies to test the packages
lexun added a commit to CodingZeal/create-react-app that referenced this pull request Dec 1, 2016
Remove bundledDependencies
a64ba0c 
Disabled because npm/npm#12834

It will also be removed upstream soon:
react#1068
alexdriaguine pushed a commit to alexdriaguine/create-react-app that referenced this pull request Jan 23, 2017
Remove bundledDependencies (react#1068)
b2a9cad 
* Remove bundledDependencies
* Change the e2e scripts to use local file dependencies instead of
  bundledDependencies to test the packages
Timer mentioned this pull request Feb 14, 2017
Closed
joaogarin referenced this pull request in jobiqo/recruiter_epiq_deps Mar 23, 2017
fix(dependencies): Remove bundled dependencies
e27dd29 
Bundled dependencies is having some weird structure on npm client implementation, so removing it to

work seamsless with npm and yarn
randycoulman pushed a commit to CodingZeal/create-react-app that referenced this pull request May 8, 2017
Remove bundledDependencies (react#1068)
1f0bb7f 
* Remove bundledDependencies
* Change the e2e scripts to use local file dependencies instead of
  bundledDependencies to test the packages
lock Bot locked and limited conversation to collaborators Jan 22, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
camwest camwest left review comments
gaearon gaearon left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed tag: internal

Projects

None yet

Milestone

0.8.0

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL