Skip to content

Merge DEV_NPM_DEPENDENCIES and PROD_NPM_DEPENDENCIES #629

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

Closed
mgechev opened this issue Mar 14, 2016 · 3 comments · Fixed by #644
Closed

Merge DEV_NPM_DEPENDENCIES and PROD_NPM_DEPENDENCIES #629

mgechev opened this issue Mar 14, 2016 · 3 comments · Fixed by #644
Labels

Comments

@mgechev
Copy link
Owner

mgechev commented Mar 14, 2016

It is confusing and redundant to have two different arrays of injectable dependencies.

The biggest current benefits of these two configuration options are:

  • Faster loading of the development build (we're able to inject bundles instead of load the individual dependencies one by one, what the prod build does).
  • We're able to use pre-minified version of some of the dependencies for the production build. This can eventually save some bandwidth if the files are minified in a special way (using Closure Compiler or uglifyjs wth mangling).

Generally only a single array of dependencies will be enough, and the difference in the treatment of the files could come from the configuration of the SystemJS builder that we use.

For some advanced builds it could be better to preserve both configuration options. My suggestion is to keep PROD_DEPENDENCIES with value null by default. If the user of the seed needs more custom behavior she can define PROD_DEPENDENCIES array and get the current behavior.

// cc @ludohenin @d3viant0ne

@joshwiens
Copy link
Contributor

I personally prefer having both configurations.

That said, defaulting to "easy mode" and documenting how to use PROD_DEPS in the advanced configuration would be easier for most users.

Quick on page doc on adding jquery, bootstrap & perhaps one popular package in an advanced configuration would cover most of the questions in the closed issues.

@ludohenin
Copy link
Collaborator

Another InjectableDependencies option :) env: 'dev | prod | both' ?
We keep both (DEV_DEP & PROD_DEP) for backward compatibility. We could also turn them into getters so we can compute whatever's needed here.

@mgechev
Copy link
Owner Author

mgechev commented Mar 15, 2016

@ludohenin getters seems like a good idea.

@d3viant0ne I like the two modes. In such case we can:

  • Keep the Angular bundles since they are essential part of the production build as well.
  • Remove the RxJS bundle since it has huge amount of operators which should be better included on demand.

This way we get:

  • Fast load of the production build in case of only a few RxJS operators included (which is the common scenario).
  • Common store for both dependencies.

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

Successfully merging a pull request may close this issue.

3 participants