fix(webapi): refactor webapi to not import util.ts directly#652
fix(webapi): refactor webapi to not import util.ts directly#652JiaLiPassion wants to merge 1 commit intoangular:masterfrom
Conversation
JiaLiPassion
commented
Feb 25, 2017
JiaLiPassion
commented
- webapi mediaquery, notification should not import methods from util.ts because it will pull all util.ts content into webapi-media-query.js and webapis-notification.js under dist
- export some zone util's method through Zone[symbol(methodName)]
- add webapis dist/min.js
mhevery
force-pushed
the
master
branch
12 times, most recently
from
b55b4ad to
f3b8885
Compare
JiaLiPassion
force-pushed
the
webapi
branch
4 times, most recently
from
ac951ee to
d416671
Compare
| name: 'mediaQuery', | ||
| invokeAddFunc: function( | ||
| addFnSymbol: any, delegate: Task|NestedEventListenerOrEventListenerObject) { | ||
| invokeAddFunc: function(addFnSymbol: any, delegate) { |
There was a problem hiding this comment.
Why are you removing the types? That does not seem right.
There was a problem hiding this comment.
The reason is I don't want to import the type from until.ts which will drag all contents from util.ts when rollup build. Just like the review here,#631 (review)
| return result; | ||
| } | ||
|
|
||
| Zone[zoneSymbol('patchEventTargetMethods')] = patchEventTargetMethods; |
There was a problem hiding this comment.
I feel very uneasy with exposing these APIs even if they are behind a symbol. What is the reasoning?
There was a problem hiding this comment.
Yes,I feel the same,the reason to expose those API in such way is
- for easier patch other non standard or 3rd party API
- don't need to import those methods from util.ts
There was a problem hiding this comment.
I think we should expose the minimal set. I will remove the rest of the exports and just keep patchEventTargetMethods
There was a problem hiding this comment.
sure, mediaQuery use patchEventTargetMethods, and Notification use patchOnProperties. I will leave those two and remove others.
3 participants
