Add options to create bundles that can be lazy loaded #35

alexthewilde · 2018-11-27T16:59:06Z

Add two new options --exclude-modules and --exclude-main-module that do exactly that.

The reasoning behind this is that I want to be able to break up Material into a lean base bundle that contains core and a few other necessary modules for the initial app load. Accompanied by more bundles that can be lazy-loaded upon demand. These lazy-loaded bundles however must not include the core modules again.

…lazy-loaded bundles.

googlebot · 2018-11-27T16:59:10Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

alexthewilde · 2018-11-27T17:00:38Z

I signed it!

googlebot · 2018-11-27T17:00:57Z

CLAs look good, thanks!

Splaktar

Thank you for the contribution! This seems like a really useful feature.

I found a couple minor things that need tweaking and I had a couple questions.

README.md

lib/cli/options.ts

README.md

devversion

LGTM. A few minor nits.

Thanks for the contribution!

README.md

devversion · 2018-12-10T17:22:28Z

README.md

 | `destination` (*)       | `string`    | Target location for the Material build.                                    |
 | `modules`               | `string[]`  | Modules that should be part of the build.<br/> All modules will be built if nothing is specified.                                                                                                           |
-| `version`               | `string`    | Version of AngularJS Material.<br/> If set to `local`, it will take the local installed AngularJS Material version from the node modules. <br/> If set to `latest`, the latest version will be downloaded.                                                           |
+| `excludeModules`               | `string[]`  | Modules that should be excluded from the build.<br/> Use to exclude e.g. core modules when building a bundle that should get lazy-loaded.                                                                                                           |


We should mention that this is not a full string match, but it also matches partially. maybe an example as you already added in code? (animate => material.core.animate)

I agree that this would be a helpful addition as well.

lib/builders/MaterialBuilder.ts

Splaktar · 2018-12-12T20:14:22Z

README.md

 | `mainFilename`          | `string`    | Name of the entry file that will be loaded to figure out the dependencies. |
 | `destinationFilename`   | `string`    | Name to be used as a base for the output files.                            |
+| `excludeModules`               | `string[]`  | Modules that should be excluded from the build.<br/> Use to exclude e.g. core modules when building a bundle that should get lazy-loaded.                                                                                                           |
+| `excludeMainModule`               | `boolean`  | Set to `true` to exclude the code for the Angular Material main module. Use when building a bundle that will get lazy-loaded and extend an already existing main module.                                                                 |


There seems to be a lot of extra space here after the name and before the type.

Please change Angular to AngularJS.

The version has been also changed from AngularJS Material to Angular Material 😄

Please fix that issue and also mention that the excludeModules option "partially" matches modules. This would be ready then!

Splaktar · 2018-12-12T20:22:24Z

README.md

 | `destination` (*)       | `string`    | Target location for the Material build.                                    |
 | `modules`               | `string[]`  | Modules that should be part of the build.<br/> All modules will be built if nothing is specified.                                                                                                           |
-| `version`               | `string`    | Version of AngularJS Material.<br/> If set to `local`, it will take the local installed AngularJS Material version from the node modules. <br/> If set to `latest`, the latest version will be downloaded.                                                           |
+| `version`               | `string`    | Version of AngularJS Material.<br/> If set to `local`, it will take the local installed Angular Material version from the node modules. <br/> If set to `latest`, the latest version will be downloaded.                                                           |


AngularJS is the correct form to use here. Please revert.

Splaktar · 2020-01-09T08:45:00Z

Just wanted to drop a note here to remind you that there are some outstanding changes needed to this PR before we can merge it. Thank you.

Adds --exclude-modules and --exlude-main-module options to allow for …

d769c41

…lazy-loaded bundles.

Fixes tslint errors.

7657e10

Splaktar self-assigned this Nov 28, 2018

Splaktar requested a review from devversion November 28, 2018 06:21

Splaktar suggested changes Nov 28, 2018

View reviewed changes

README.md Outdated Show resolved Hide resolved

lib/cli/options.ts Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

alexthewilde added 4 commits December 10, 2018 17:42

Use term AngularJS instead of Angular.

a7c542b

Merge branch 'master' into exclude_core

9197c3a

Use camelCase for options docs.

f300e95

Resolved merge conflict.

0896bbe

devversion approved these changes Dec 10, 2018

View reviewed changes

Moved options docs.

8d1d998

Splaktar reviewed Dec 12, 2018

View reviewed changes

Splaktar added P3: important - in progress labels Dec 12, 2018

Splaktar reviewed Dec 12, 2018

View reviewed changes

Splaktar added this to the Backlog milestone Oct 30, 2020

Add options to create bundles that can be lazy loaded #35

Are you sure you want to change the base?

Add options to create bundles that can be lazy loaded #35

Uh oh!

Conversation

alexthewilde commented Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Nov 27, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

alexthewilde commented Nov 27, 2018

Uh oh!

googlebot commented Nov 27, 2018

Uh oh!

Splaktar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devversion left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devversion Dec 10, 2018

Choose a reason for hiding this comment

Uh oh!

Splaktar Dec 12, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Splaktar Dec 12, 2018

Choose a reason for hiding this comment

Uh oh!

Splaktar Dec 12, 2018

Choose a reason for hiding this comment

Uh oh!

devversion Dec 12, 2018

Choose a reason for hiding this comment

Uh oh!

Splaktar Dec 12, 2018

Choose a reason for hiding this comment

Uh oh!

Splaktar commented Jan 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexthewilde commented Nov 27, 2018 •

edited

Loading

devversion left a comment •

edited

Loading