feat: make codebase more modular so that only parts of it can be loaded #748
Conversation
|
@JiaLiPassion I am working on speeding up the loading of the zone as well as making the loading more modular. This is a start, would love to have your feedback. |
|
@mhevery , got it, thank you for involving me, I will look into it. |
| */ | ||
| originalStack?: string; | ||
| } | ||
|
|
||
| /** @internal */ | ||
| type AmbientZone = Zone; | ||
| /** @internal */ | ||
| type AmbientZoneDelegate = ZoneDelegate; | ||
|
|
||
| const Zone: ZoneType = (function(global: any) { |
There was a problem hiding this comment.
@mhevery , Should we separate the Zone interface and implementation into separate files?
There was a problem hiding this comment.
We could. What would be the advantage?
There was a problem hiding this comment.
Just to clarify the interface and implementation and reduce the zone.ts size, it is a little big, user may easily check the API from the interface file.
But we have the zone.js.d.ts file, so I think it is also ok in current way.
| }; | ||
| }; | ||
| const patches: {[key: string]: any} = {}; | ||
| const _api: _ZonePrivate = { |
There was a problem hiding this comment.
should we expose those API in two kinds
-
util method as
symbol
other module can just use it. -
listeners as
consoleError
other module can add listener to it. I think maybe there are more modules want to handle unhandled microtask error for example.
There was a problem hiding this comment.
-
I want to keep in hidden so that you can only get to it from the callback.
-
We could do that, but since we only have one place which needs it so far, I would prefer to have a use case before we complicate things.
| function findPendingTask(target: any) { | ||
| const pendingTask: Task = target[XHR_TASK]; | ||
| return pendingTask; | ||
| Zone.__load_patch('timers', (global: any, Zone: ZoneType, api: _ZonePrivate) => { |
There was a problem hiding this comment.
I like this one!!!, this is much more clear!!!
We should also patch other non-standard libraries in this way, such as
- Notification
- MediaQuery
- 3rd party promise
...
There was a problem hiding this comment.
Yes, that is my plan, just have not gotten to it all.
lib/common/utils.ts
Outdated
| @@ -133,7 +133,7 @@ export function patchProperty(obj: any, prop: string) { | |||
| // the onclick will be evaluated when first time event was triggered or | |||
| // the property is accessed, https://github.com/angular/zone.js/issues/525 | |||
| // so we should use original native get to retrieve the handler | |||
| let value = originalDescGet.apply(this); | |||
| let value = originalDescGet && originalDescGet.apply(this); | |||
| value = desc.set.apply(this, [value]); | |||
| @@ -648,6 +651,17 @@ const Zone: ZoneType = (function(global: any) { | |||
| return _currentTask; | |||
| }; | |||
|
|
|||
| static __load_patch(name: string, fn: _PatchFn): void { | |||
There was a problem hiding this comment.
should we allow 3rd party add their own patch?
If so, we may need to consider which Zone Private API should we expose.
There was a problem hiding this comment.
I would prefer to keep this private to zone for now. Would love to see some use cases before we expose it.
| } | ||
|
|
||
| function drainMicroTaskQueue() { | ||
| const showError: boolean = !(Zone as any)[__symbol__('ignoreConsoleErrorUncaughtError')]; |
There was a problem hiding this comment.
Do we need to organize all symbol to some const definition block or separate file?
It is a little hard to find what are defined or used...
One more thing, should we define some ZoneConfiguration to contains all Zone settings
such as
{
ignoreConsoleErrorUncaughtError: true
}
or
interface ZoneSettings {
ignoreConsoleErrorUncaughtError: boolean
}
There was a problem hiding this comment.
Yes that sounds like a good idea. Perhaps we could do that as a second pass after this?
lib/zone.ts
Outdated
| if ((Zone as any)[__symbol__('ignoreConsoleErrorUncaughtError')]) { | ||
| return; | ||
| } | ||
| _api.consoleError(showError, error); |
There was a problem hiding this comment.
should we define this API as
processUnhandleMicrotaskError(...)
or something like that?
There was a problem hiding this comment.
good, idea changed.
mhevery
force-pushed
the
modular
branch
3 times, most recently
from
f25033b
to
5258a1d
Compare
|
I just realised this my comment should be moved into an issue, so, I here it is - #991 |
No description provided.