Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor(build): No longer building AMD, umd bundle is Rx.js #1871

Merged
merged 1 commit into from Aug 10, 2016
Merged

refactor(build): No longer building AMD, umd bundle is Rx.js #1871

merged 1 commit into from Aug 10, 2016

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Aug 9, 2016

This is still using browserify for bundling. This is just the minimal change to close #1678.

If TSC is now providing a built-in way of bundling and UMD output, we should be using that, but for now, browserify is fine.

BREAKING CHANGE: AMD modules are no longer built and supported
BREAKING CHANGE: Rx.umd.js and the like are now just Rx.js

Closes #1678

@benlesh
Copy link
Member Author

benlesh commented Aug 9, 2016

attn: @kwonoj @jayphelps

Please review thoroughly. Since this change isn't one where it's easy to detect breakage via automation.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.068% when pulling 6b9f021 on blesh:no-amd-just-umd into f3d4250 on ReactiveX:master.

kwonoj
kwonoj reviewed Aug 9, 2016
View reviewed changes
README.md


5.0.0-beta.11 or higher:

https://npmcdn.com/@reactivex/rxjs/dist/global/Rx.umd.js
Copy link
Member

Choose a reason for hiding this comment

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

b.11 or higher may need to point Rx.js instead of Rx.umd.js?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I believe this should be changed to 5.0.0-beta.12 or higher need to point to Rx.js cause the previous section includes beta.11 (the current release) and correctly points to Rx.umd.js

Copy link
Member

Choose a reason for hiding this comment

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

yes, that seems more correct. Maybe just align with installation.md, saying After beta.11?

@jayphelps
Copy link
Member

Can we change the BREAKING CHANGE: AMD modules are no longer built and supported message to be more specific? It currently sounds like AMD is no longer supported at all, but in fact most commonly people are likely consuming the UMD module which has AMD support. Basically, we're no longer packaging AMD modules for each file, instead only supporting AMD as part of the single UMD build.

@jayphelps
Copy link
Member

jayphelps commented Aug 10, 2016 •
edited

  • Make breaking change message not sound like there's zero support for AMD
  • Correct/wordsmith the umd link for 5.0.0-beta.12 or higher in README.md

Otherwise looks good.

@benlesh
refactor(build): No longer building AMD modules, UMD bundle is now Rx.js
82813f0 
BREAKING CHANGE: Individual AMD modules are no longer being built. AMD
users my use the single UMD bundle output.
BREAKING CHANGE: UMD bundled output is no longer named like `Rx.umd.js`, the `.umd` part has been removed, and it's just `Rx.js`

Closes #1678
@benlesh
Copy link
Member Author

benlesh commented Aug 10, 2016

@jayphelps updates made.

@kwonoj
Copy link
Member

kwonoj commented Aug 10, 2016

change looks good to me.

@coveralls
Copy link

coveralls commented Aug 10, 2016 •
edited

Coverage Status

Coverage remained the same at 97.068% when pulling 6b9f021 on blesh:no-amd-just-umd into f3d4250 on ReactiveX:master.

@jayphelps
Copy link
Member

:shipit:

@jayphelps jayphelps merged commit 09ef83c into ReactiveX:master Aug 10, 2016
jlesquembre added a commit to jlesquembre/rxjs that referenced this pull request Aug 12, 2016
@jlesquembre
docs(esdoc.json): related to ReactiveX#1871, Rx.umd.js is now just Rx.js
0390f04
@jlesquembre jlesquembre mentioned this pull request Aug 12, 2016
Merged
benlesh pushed a commit that referenced this pull request Aug 12, 2016
@jlesquembre @benlesh
docs(esdoc.json): related to #1871, Rx.umd.js is now just Rx.js (#1878)
a2b2f3d
@jayphelps jayphelps mentioned this pull request Sep 6, 2017
Open
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rx.umd.js vs Rx.js vs AMD
4 participants
@benlesh @coveralls @jayphelps @kwonoj