-
Notifications
You must be signed in to change notification settings - Fork 408
feat(Zone): check global for rootZoneSpec #521
Conversation
As described in comment of #513, I am not sure my thinking is correct or not, but I think we shouldn't allow access ZoneSpec at any level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
const symbolRootZoneSpec = '__rootZoneSpec__' | ||
var rootZone: Zone = new Zone(null, null); | ||
if (global[symbolRootZoneSpec]) { | ||
rootZone = <Zone>rootZone.fork(global[symbolRootZoneSpec]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we delete the rootZoneSpec after create rootZone?
const symbolRootZoneSpec = '__rootZoneSpec__'
var rootZone: Zone = new Zone(null, null);
if (global[symbolRootZoneSpec]) {
rootZone = <Zone>rootZone.fork(global[symbolRootZoneSpec]);
delete global[symbolRootZoneSpec];
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should so that we don't clutter global space.
I can't see any reason not to.
@@ -952,7 +952,14 @@ const Zone: ZoneType = (function(global: any) { | |||
const symbolPromise = __symbol__('Promise'); | |||
const symbolThen = __symbol__('then'); | |||
|
|||
let _currentZoneFrame = new ZoneFrame(null, new Zone(null, null)); | |||
const symbolRootZoneSpec = '__rootZoneSpec__' | |||
var rootZone: Zone = new Zone(null, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use let instead of var
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Could you please rerun the tests on travis? |
Sorry, I have no permission to rerun the test on travis, the error seems to be timeout, it should be ok to run again, in your PR, a ';' is missing in line 955, maybe you can commit the ';' and it will trigger a new travis test run. |
@@ -952,7 +952,14 @@ const Zone: ZoneType = (function(global: any) { | |||
const symbolPromise = __symbol__('Promise'); | |||
const symbolThen = __symbol__('then'); | |||
|
|||
let _currentZoneFrame = new ZoneFrame(null, new Zone(null, null)); | |||
const symbolRootZoneSpec = '__rootZoneSpec__' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more, missing ';' here.
The build should be ok now, to trigger tavis build again without adding more useless commit, you may rollback your commit by
and redo the last change (add semicolon) and commit again. |
be8c7d6
to
f8ffd60
Compare
just rebased, tests are passing now. can we merge this one? |
Sorry, but this is by design. RootZone should be indistinguishable for no zone at all. This requirement is here so that if we standardize the Zones in TC39, the browser vendors will not have to pay the cost for all of the applications. All zones must be entered explicitly. |
closes #513