Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

Working on rollup bundling#39

Closed
thelgevold wants to merge 5 commits intoangular:masterfrom
thelgevold:rollup
Closed

Working on rollup bundling#39
thelgevold wants to merge 5 commits intoangular:masterfrom
thelgevold:rollup

Conversation

@thelgevold
Copy link
Contributor

WIP: DO NOT MERGE!

This is a wip PR to get some feedback on the approach.

TODOS:

  1. Fix import statements to avoid using a path rewrite plugin in rollup
  2. Figure out a way to streamline the paths to avoid long path references in the rollup script.

@thelgevold thelgevold force-pushed the rollup branch 2 times, most recently from 252959a to ba276eb Compare December 19, 2017 20:53
@thelgevold
Copy link
Contributor Author

Simplified some of the "path-ing"

index.html Outdated

<app-component></app-component>

<script src="bazel-out/darwin-fastbuild/bin/src/rollup.runfiles/angular_bazel_example/dist/bundle.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this example ought to come with a <10 line express server so that the bundle can be served from the same URL as it would be under ts_devserver

Copy link

Choose a reason for hiding this comment

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

My trick is just to have a ts_devserver which doesn't actually produce a bundle itself but instead lists the production bundle as a static resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added configuration to match the url of ts_devserver

README.md Outdated
Experimental Production bundling with Rollup:

```bash
$ bazel run src:rollup
Copy link
Contributor

Choose a reason for hiding this comment

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

thinking out loud: we'll need to set users expectations about whether we'd ever support code splitting and lazy loading with a rollup setup. It's probably hard and not worth it since we'd be creating a new capability in that toolchain?
/cc @robwormald

"protractor": "5.2.2",
"typescript": "2.6.2"
"typescript": "2.6.2",
"rollup": "0.52.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

when this shapes up more, the user shouldn't be exposed to the rollup npm dependencies

rollup-bundle.js Outdated

class NormalizePaths {
// TODO Fix import statements in ngfactories to avoid this path plugin
resolveId(importee, importer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if we didn't have to patch filepaths, is there a way to just call the rollup CLI without having our own main like this?

src/rollup.bzl Outdated
def _rollup(ctx):

args = ["--config", "../../../../execroot/angular_bazel_example/rollup.config.js"]
args += ["--output.file", ctx.outputs.build.path]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to avoid path navigation like this, but I am not sure how to best navigate the folder structure or pass files along to rules.

@thelgevold
Copy link
Contributor Author

The bundling is a three step process (Rollup, TS downleveling, Uglify). I currently run this as three different bazel commands, but I am assuming we could simplify
bazel build //src:bundle && bazel build //src:es5 && bazel run //src:uglify to a single command where the output from the previous rule is passed to the next?

I have not figured out how to do that in Bazel though

@thelgevold
Copy link
Contributor Author

Updated the url to match the url of ts_devserver

@thelgevold thelgevold force-pushed the rollup branch 11 times, most recently from 77dc888 to a2d790e Compare December 22, 2017 15:14
s

s

s

s

s

s

s

s

s

s

s

s

s

s

s

s

s

s

s
@alexeagle
Copy link
Contributor

We worked through this together, your fork is the master copy right now. Let's design how to land the real rollup rule

@alexeagle alexeagle closed this Jan 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants