Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

RxJS patch is not closure safe #863

Closed
mhevery opened this issue Aug 2, 2017 · 1 comment · Fixed by #848
Closed

RxJS patch is not closure safe #863

mhevery opened this issue Aug 2, 2017 · 1 comment · Fixed by #848
Assignees
@JiaLiPassion

Comments

@mhevery
Copy link
Contributor

mhevery commented Aug 2, 2017

  1. Assignment into imported constructor is not allowed: https://github.com/angular/zone.js/blob/master/lib/rxjs/rxjs.ts#L75
  2. We can’t use string literals here: https://github.com/angular/zone.js/blob/master/lib/rxjs/rxjs.ts#L230 since they don’t minify in closure

The fix for #1 is https://github.com/trxcllnt/rxjs/blob/53eddb1cca1228fe6d81aa6bec0ee0a57c8f4664/src/add/zone.ts#L60-L74 The trick is to intercept the setter to have side-effects. This way we don’t need to replace the constructor.
@JiaLiPassion
Copy link
Collaborator

@mhevery , I have modified this one in #848, please review.

@JiaLiPassion JiaLiPassion mentioned this issue Aug 4, 2017
Merged
mhevery pushed a commit that referenced this issue Aug 8, 2017
@JiaLiPassion @mhevery
fix(rxjs): fix #863, fix asap scheduler issue, add testcases (#848)
cbc58c1 
* fix(rxjs): fix asap scheduler issue, add testcases

* patch defer, add cases for concat/defer

* add testcases: from/forkJoin

* add cases: fromEvent/fromPromise/interval

* add merge/never/of/range/throw/timer/zip

* add audit~delay

* staging changes

* fix test cases

* add notification test

* add merge/map/collections

* add remained cases

* patch multicast

* use _subscriber setter instead of patching Observable constructor

* refactor

* update gulpfiles

* check proto null

* add null check if operators are not loaded

* remove not necessary karma settings
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@JiaLiPassion JiaLiPassion

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(rxjs): fix #863, fix asap scheduler issue, add testcases JiaLiPassion/zone.js
2 participants
@mhevery @JiaLiPassion