-
Notifications
You must be signed in to change notification settings - Fork 409
Some application break on specific mobile device or environment. #1098
Conversation
Pull request number #1041 break app in some mobile browser that Promise.then is not writeable. (We tested on Samsung Galaxy A5) I agree #1041 code change because it is not writable. But i think just break some application without any warning message is not right. So i add fallback code and display some wraning message for that.
} else { | ||
console.error(`Cannot found Promise#then method. Please add polyfill to using \`Promise\`. If you already did that. Perhaps Promise#then is not writeable. If you want to use it. add following codes to your \`polyfill.ts\` after \`import 'zone.js/dist/zone'\` | ||
|
||
if (!(Promise.prototype as any)['__zone_symbol__then']) { |
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.
the code should not be added here, because it means the current microTask will not be scheduled. If you think this logic need to be handled, you need to add the logic to patchThen
method, but I don't think we need to handle old browser like chrome 34
.
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.
this line is not code. just part of template string.
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. got it, in fact, I forget I already fixed this one in #1079. So please wait for the next release.
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.
No #1079 is not initialize Promise.__zone_symbol__then
because Promise.then
is not patchable OK?
~~The problem is https://github.com/angular/zone.js/pull/1098/files/d4ac887fd112c3fde3a167223c42ecbac76c1f14#diff-dd469785fca8680a5b33b1e81c5cfd91R1276 ~~
nativeMicroTaskQueuePromise
is Promise
and symbolThen
is '__zone_symbol__then. so Invoking
Promise. __zone_symbol__then` is always throwing error.
But some case. Promise.then is not patchable but the error message only undefined is not function
and no other error message, no error stack trace. in this case It's very hard to find this problem.
It is not problem of only old browser. If some library set Promise.then
to not patchable. developer will be face this problem and searching what is the matter of this error message.
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.
@JiaLiPassion Oh #1079 is exactly i want. I will wating next release. 👍
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, sure, thank you for reporting the issue.
Duplicate #1079. |
Pull request number #1041 break app in some mobile browser that Promise.then is not writeable. (We tested on Samsung Galaxy A5) and I agree #1041 code change because it is not writable.
But I think just break some application without any warning message is not right. It's very hard to debugging because error message is
undefined is not function
and no any useful information like message or stacktrace.So i add fallback code and display some warning message for that. If you don't want my PR. please suggest other way to notify the user.