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

perf(all): improve rxjs imports #29

Merged
merged 1 commit into from
Aug 19, 2017
Merged

perf(all): improve rxjs imports #29

merged 1 commit into from
Aug 19, 2017

Conversation

trotyl
Copy link
Contributor

@trotyl trotyl commented Aug 16, 2017

import {} from 'rxjs/Rx' will include the entire RxJS library.

Fixes #50 .
Relates to #18 .

@trotyl trotyl changed the title perf(all): improve rxjs imports [WIP] perf(all): improve rxjs imports Aug 16, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 52.183% when pulling 508245d on trotyl:rxjs-import into 32234e4 on NG-ZORRO:master.

Copy link
Contributor

@LinboLen LinboLen left a comment

Choose a reason for hiding this comment

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

mark a comment, need to review

@@ -144,7 +145,7 @@ export class NzSubMenuComponent implements OnInit, OnDestroy, AfterViewInit {
}

ngOnInit() {
this._$mouseSubject.debounceTime(300).subscribe((data: boolean) => {
debounceTime.call(this._$mouseSubject, 300).subscribe((data: boolean) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

should wrap with runOutsideAngular.

runOutsideAngular means that every hooked global async func will restore.

baraly introduce rxjs without ngZone check is not recommend.

if use runOutsideAngular , the detectChange does't work, can force trigger detect change with ChangeDetectorRef injected in contructor as private changeDectorRef: ChangeDetectorRef then call this.changeDectorRef.detectChanges().

hope to help you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could be done in a separate request, since this is only for fixing the rxjs imports, shouldn't change any other part of the lib.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 52.183% when pulling a8b16e3 on trotyl:rxjs-import into 32234e4 on NG-ZORRO:master.

@trotyl trotyl changed the title [WIP] perf(all): improve rxjs imports perf(all): improve rxjs imports Aug 16, 2017
@vthinkxie vthinkxie merged commit f3aa2cb into NG-ZORRO:master Aug 19, 2017
@FunClub
Copy link

FunClub commented Aug 21, 2017

import {} from 'rxjs/Rx'请问这句代码应该添加到那里

@trotyl
Copy link
Contributor Author

trotyl commented Aug 21, 2017

@FunClub Nowhere if you're writing front-end.

@trotyl trotyl deleted the rxjs-import branch August 21, 2017 03:01
@mango-csl
Copy link

请问 NG-ZORRO怎么按需加载,以减小打包体积

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants