Skip to content

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Nov 18, 2016

Installing dependencies "global-style" creates a top-level directory in node_modules for each direct dependency, and then creates a flat tree of dependencies inside each of those.

I prefer this method because it prevents accidentally depending on a dependency of a dependency, and makes it easier to find dependencies in node_modules.

Unfortunately the node_modules directory will be a little deeper.

Installing dependencies "global-style" creates a top-level directory in `node_modules` for each direct dependency, and then creates a flat tree of dependencies inside each of those.

I prefer this method because it prevents accidentally depending on a dependency of a dependency, and makes it easier to find dependencies in `node_modules`.

Unfortunately the `node_modules` directory will be a little deeper.
@epixa epixa changed the title [npm] install deps "global-sytle" [npm] install deps "global-style" Nov 18, 2016
@epixa
Copy link
Contributor

epixa commented Nov 18, 2016

Can you do a rough analysis on path lengths in Kibana right now with this enabled?

@spalger
Copy link
Contributor Author

spalger commented Nov 18, 2016

Interestingly enough, it doesn't effect the longest path in the dependencies:

191 characters, and it's installed with the same path regardless of global-style

node_modules/npm/node_modules/npm-registry-client/node_modules/npmlog/node_modules/gauge/node_modules/string-width/node_modules/is-fullwidth-code-point/node_modules/number-is-nan/package.json

to find the longest path:

find node_modules |awk '{print length, $0}'|sort -nr|head -1

@spalger
Copy link
Contributor Author

spalger commented Nov 18, 2016

Also interesting, the same is true with yarn

@spalger
Copy link
Contributor Author

spalger commented Nov 18, 2016

If I install just production dependencies I get 126 in normal mode, and 130 in global-style mode

# deduped tree 126
node_modules/@elastic/kibana-ui-framework/src/guide/views/local_nav/local_nav_menu_item_states/local_nav_menu_item_states.html

# global-style 130
node_modules/bunyan/node_modules/dtrace-provider/build/Release/.deps/Release/obj.target/DTraceProviderBindings/dtrace_provider.o.d

So, it actually doesn't have much impact on the bottom line

@tbragin tbragin added the Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// label Nov 19, 2016
@epixa
Copy link
Contributor

epixa commented Nov 21, 2016

I can't think of any problems this would create, so I guess we should just see it in action. I do like the result of the change.

LGTM

@spalger Can you do the same change for x-pack as well?

@spalger spalger merged commit e62c48d into master Nov 21, 2016
elastic-jasper added a commit that referenced this pull request Nov 21, 2016
Backports PR #9136

**Commit 1:**
[npm] install deps "global-sytle"

Installing dependencies "global-style" creates a top-level directory in `node_modules` for each direct dependency, and then creates a flat tree of dependencies inside each of those.

I prefer this method because it prevents accidentally depending on a dependency of a dependency, and makes it easier to find dependencies in `node_modules`.

Unfortunately the `node_modules` directory will be a little deeper.

* Original sha: 95103f0
* Authored by Spencer <[email protected]> on 2016-11-18T16:43:54Z
* Committed by GitHub <[email protected]> on 2016-11-18T16:43:54Z
epixa pushed a commit that referenced this pull request Nov 21, 2016
Backports PR #9136

**Commit 1:**
[npm] install deps "global-sytle"

Installing dependencies "global-style" creates a top-level directory in `node_modules` for each direct dependency, and then creates a flat tree of dependencies inside each of those.

I prefer this method because it prevents accidentally depending on a dependency of a dependency, and makes it easier to find dependencies in `node_modules`.

Unfortunately the `node_modules` directory will be a little deeper.

* Original sha: 95103f0
* Authored by Spencer <[email protected]> on 2016-11-18T16:43:54Z
* Committed by GitHub <[email protected]> on 2016-11-18T16:43:54Z
@spalger spalger deleted the npm-global-style branch November 23, 2016 23:03
epixa added a commit to epixa/kibana that referenced this pull request Nov 29, 2016
This change, which was originally introduced in elastic#9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.
@epixa epixa added reverted and removed review labels Nov 29, 2016
epixa added a commit that referenced this pull request Nov 29, 2016
This change, which was originally introduced in #9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.
elastic-jasper added a commit that referenced this pull request Nov 29, 2016
Backports PR #9256

**Commit 1:**
Removes "global-style" setting for npm

This change, which was originally introduced in #9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.

* Original sha: bdf4c68
* Authored by Court Ewing <[email protected]> on 2016-11-29T20:29:43Z
elastic-jasper added a commit that referenced this pull request Nov 29, 2016
Backports PR #9256

**Commit 1:**
Removes "global-style" setting for npm

This change, which was originally introduced in #9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.

* Original sha: bdf4c68
* Authored by Court Ewing <[email protected]> on 2016-11-29T20:29:43Z
epixa pushed a commit that referenced this pull request Nov 29, 2016
Backports PR #9256

**Commit 1:**
Removes "global-style" setting for npm

This change, which was originally introduced in #9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.

* Original sha: bdf4c68
* Authored by Court Ewing <[email protected]> on 2016-11-29T20:29:43Z
epixa pushed a commit that referenced this pull request Nov 29, 2016
Backports PR #9256

**Commit 1:**
Removes "global-style" setting for npm

This change, which was originally introduced in #9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.

* Original sha: bdf4c68
* Authored by Court Ewing <[email protected]> on 2016-11-29T20:29:43Z
@spalger spalger changed the title [npm] install deps "global-style" REVERTED in #9256 - [npm] install deps "global-style" Dec 2, 2016
@epixa epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
Backports PR elastic#9136

**Commit 1:**
[npm] install deps "global-sytle"

Installing dependencies "global-style" creates a top-level directory in `node_modules` for each direct dependency, and then creates a flat tree of dependencies inside each of those.

I prefer this method because it prevents accidentally depending on a dependency of a dependency, and makes it easier to find dependencies in `node_modules`.

Unfortunately the `node_modules` directory will be a little deeper.

* Original sha: 95103f0
* Authored by Spencer <[email protected]> on 2016-11-18T16:43:54Z
* Committed by GitHub <[email protected]> on 2016-11-18T16:43:54Z

Former-commit-id: 269ee06
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
Backports PR elastic#9256

**Commit 1:**
Removes "global-style" setting for npm

This change, which was originally introduced in elastic#9136, resulted in much
larger installs for Kibana (50MB larger in dev mode), which isn't a
reasonable tradeoff for the convenience that global-style provides.

* Original sha: bdf4c68
* Authored by Court Ewing <[email protected]> on 2016-11-29T20:29:43Z

Former-commit-id: 9b43bdc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reverted Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v5.1.1 v6.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants