Build bazel umds #4309
Merged
Build bazel umds #4309
Conversation
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
alan-agius4
force-pushed
the
build_bazel_umds
branch
from
fb6f53b to
6a6cd77
Compare
Generated by 🚫 dangerJS |
alan-agius4
force-pushed
the
build_bazel_umds
branch
from
6a6cd77 to
54d9f29
Compare
Pull Request Test Coverage Report for Build 7674
💛 - Coveralls |
This was referenced Oct 31, 2018
alexeagle
reviewed
Nov 6, 2018
Contributor
Author
|
I am open for both as well. Depends on what they prefer. I can split them
in several Pr if it’s preferred.
…On Wed, 07 Nov 2018 at 15:07, Alex Eagle ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tools/rollup-bundle.js
<#4309 (comment)>:
> + rollupNodeResolve({
+ jsnext: true,
+ }),
+ rollupInject({
+ exclude: 'node_modules/**',
+ modules: _.mapValues(tslib, function (value, key) {
+ return ['tslib', key];
+ }),
+ }),
+ ],
+ }).then(function (bundle) {
+ return bundle.generate({
+ format: 'umd',
+ name: 'rxjs',
+ amd: {
+ id: 'rxjs'
I see - depending on what the rxjs maintainers prefer, it might be easier
to land in two PRs: update rollup version, and separately change the id and
remove Bazel files
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4309 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQv-WklybJGYfuO545hOsvxWbovtA4Fnks5usvcXgaJpZM4YD1Rv>
.
|
pertrai1
reviewed
Nov 12, 2018
Contributor
pertrai1
left a comment
pertrai1
left a comment
There was a problem hiding this comment.
I see it better for the changelog if there were 2 PR's:
- update rollup version
- change the id and remove Bazel files
Contributor
Author
|
@pertrai1, there are 2 commits, and thus there is no difference in the changelog, apart from that these are |
Contributor
|
Thinking a bit more: removing the Bazel files is a breaking change and should probably land later? |
Contributor
|
At least the rollup upgrade should be it's own commit |
alan-agius4
force-pushed
the
build_bazel_umds
branch
from
54d9f29 to
013cd20
Compare
Contributor
Author
|
Yeah I think I makes sense to do it afterwards. Updated. |
alan-agius4
force-pushed
the
build_bazel_umds
branch
from
013cd20 to
7cd99e2
Compare
alexeagle
approved these changes
Nov 20, 2018
alan-agius4
force-pushed
the
build_bazel_umds
branch
2 times, most recently
from
237c64d to
06cdd89
Compare
This is required so that we can use named UMDs with named AMD, since these are required in Bazel (ts_devserver). This feature doesn't work in the current version rollup. Rollup's API also changed quite a bit, thus the large refactoring.
This is because otherwise sourcemaps will not be mapped for UMD bundles
This solved the issue is that to run in devmode with Bazel (ts_devserver) you need a named named AMD bundle (or a UMD bundle with named AMD). RXJS doesn’t ship one of these so we’re forced to build rxjs from source so get named AMD source files
alan-agius4
force-pushed
the
build_bazel_umds
branch
from
06cdd89 to
e174615
Compare
Contributor
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR adds AMD ids in UMD bundles, sourceMapUrl in UMD bundle, and remove Bazel files
//cc @alexeagle & @gregmagolan
Ps: I can split this into multiple PR, if you'd like.