fix: Promise polyfill for zone #1178
Conversation
Fixes evaluateAsync behavior when [zone.js](https://github.com/angular/zone.js) has mucked with Promise. Adds zone.js to the smoke tests so regression won't happen again. Addresses #1177.
| @@ -143,15 +143,19 @@ class Driver { | |||
| ); | |||
|
|
|||
| this.sendCommand('Runtime.evaluate', { | |||
| // We need to wrap the raw expression for several purposes | |||
| // We need to expliticly wrap the raw expression for several purposes: | |||
There was a problem hiding this comment.
oops, done :)
Looks like you didn't have to modify @patrickhulce, so this could work this time? |
|
yeah true I didn't this time around, will fix |
| const thirdPartyPath = '../../../lighthouse-core/third_party'; | ||
| absoluteFilePath = path.join(__dirname, `${thirdPartyPath}/promise-polyfill/promise.js`); | ||
| // We bring in an aggressive Promise polyfill (zone) to ensure we don't still fail. | ||
| const zonePath = '../../../node_modules/zone.js'; |
There was a problem hiding this comment.
@ebidel not sure what you're requesting here, to change zonePath to nodeModule path?
There was a problem hiding this comment.
OH nvm. This is probably where it gets installed. Just looks funny from a reviewer's perspective :)
| .then(_ => ${expression}) | ||
| .catch(${wrapRuntimeEvalErrorInBrowser.toString()}) | ||
| .then(resolve); | ||
| }); |
There was a problem hiding this comment.
I tried a few permutations of this and it looks like this is about the only decent one that works :) Reason in #1173 (comment).
For anyone following along, the nice thing about this form is that it lets the polyfill handle resolving to a value (or error), and then all we have to assume is that our __nativePromise constructor is intact, not our earlier assumption that its prototype chain had also not been altered
…Chrome#1178) * Fix `driver.evaluateAsync` behavior when [zone.js](https://github.com/angular/zone.js) has mucked with Promise. * Add zone.js to the smoke tests so regression won't happen again. * Remove old testing promise-polyfill
R: @brendankenny
Fixes evaluateAsync behavior when zone.js has mucked with Promise.
Adds zone.js to the smoke tests so regression won't happen again.
Addresses #1173