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.

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

build(npm): update rxjs to 2.0.0-beta.6 #8047

Closed
wants to merge 1 commit into from

Conversation

jeffbcross
Copy link
Contributor

Note: this contains two commits from #8003 to make sure Travis will be green.

@jeffbcross
Copy link
Contributor Author

I think I need to fix a couple of typings in http

@jeffbcross
Copy link
Contributor Author

There are obscure Travis failures in environments I don't have time to troubleshoot right now, so please don't merge this unless someone can make it green.

@jeffbcross jeffbcross added state: blocked and removed action: merge The PR is ready for merge by the caretaker pr_state: LGTM labels Apr 20, 2016
@jeffbcross
Copy link
Contributor Author

I'm going to try reproducing and fixing the Windows and Safari issues now

@jeffbcross
Copy link
Contributor Author

@Blesh had the idea that our Symbol problems are probably the result of RxJS no longer polyfilling Symbol as it did in beta.2, and our code in facade/lang.ts was assuming that Symbol had been polyfilled when calling isPresent(Symbol) in lang.ts:

if (isPresent(Symbol) && isPresent(Symbol.iterator)) {

I changed that line to if ('Symbol' in globalScope && isPresent(Symbol.iterator)) {

@jeffbcross jeffbcross added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: blocked labels Apr 21, 2016
@jeffbcross jeffbcross added the action: merge The PR is ready for merge by the caretaker label Apr 21, 2016
@jeffbcross jeffbcross assigned robertmesserle and unassigned alxhub Apr 21, 2016
@jeffbcross jeffbcross added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 21, 2016
@jeffbcross
Copy link
Contributor Author

Got verbal LGTM from @alxhub

@robertmesserle this will require updating internal version of Rx, which I can help with. It'd be great to get this landed before our next beta release.

@benlesh
Copy link
Contributor

benlesh commented Apr 21, 2016

@jeffbcross going forward, if you need to access Symbol.observable you'll want to use this module: https://www.npmjs.com/package/symbol-observable

@rkirov
Copy link
Contributor

rkirov commented Apr 25, 2016

This is blocked on google internal ts1.9 upgrade. Will revisit after ng-conf.

@benlesh
Copy link
Contributor

benlesh commented Apr 25, 2016

Just noticed this:

update rxjs to 2.0.0-beta.6

That would be a 3 major version downgrade.

@jeffbcross
Copy link
Contributor Author

I'm going to rebase and get this thing ready for merge. We're not going to wait on G3. This is blocking some offline compiler work now.

@jeffbcross
Copy link
Contributor Author

Thx @Blesh fixed commit message :)

@jeffbcross jeffbcross assigned rkirov and unassigned robertmesserle Apr 26, 2016
@jeffbcross
Copy link
Contributor Author

READY TO MERGE!

@mary-poppins
Copy link

Merging PR #8047 on behalf of @rkirov to branch presubmit-rkirov-pr-8047.

@PatrickJS
Copy link
Member


:

@mhevery mhevery closed this in b62bccf Apr 26, 2016
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants