Skip to content

Commit

Permalink
fix: Promise polyfill for zone
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
patrickhulce committed Dec 19, 2016
1 parent 7c0d6aa commit 44a634f
Show file tree
Hide file tree
Showing 5 changed files with 1,642 additions and 266 deletions.
2 changes: 1 addition & 1 deletion lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,6 @@ <h2>Do better web tester page</h2>
}
</script>

<script src="/promise_polyfill.js"></script>
<script src="/zone.js"></script>
</body>
</html>
6 changes: 3 additions & 3 deletions lighthouse-cli/test/fixtures/static-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ function requestHandler(request, response) {
const queryString = requestUrl.search;
let absoluteFilePath = path.join(__dirname, filePath);

if (filePath === '/promise_polyfill.js') {
if (filePath === '/zone.js') {
// evaluateAsync previously had a bug that LH would fail if a page polyfilled Promise.
// We bring in a third-party Promise polyfill to ensure we don't still fail.
// We bring in an aggressive Promise polyfill (zone) to ensure we don't still fail.
const thirdPartyPath = '../../../lighthouse-core/third_party';
absoluteFilePath = path.join(__dirname, `${thirdPartyPath}/promise-polyfill/promise.js`);
absoluteFilePath = path.join(__dirname, `${thirdPartyPath}/zone/zone.js`);
}

fs.exists(absoluteFilePath, fsExistsCallback);
Expand Down
14 changes: 9 additions & 5 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
// 1. Ensure that the expression will be a native Promise and not a polyfill/non-Promise.
// 2. Ensure that errors captured in the Promise are converted into plain-old JS Objects
// 2. Ensure that errors in the expression are captured by the Promise.
// 3. Ensure that errors captured in the Promise are converted into plain-old JS Objects
// so that they can be serialized properly b/c JSON.stringify(new Error('foo')) === '{}'
expression: `(function wrapInNativePromise() {
const __nativePromise = window.__nativePromise || Promise;
return __nativePromise.resolve()
.then(_ => ${expression})
.catch(${wrapRuntimeEvalErrorInBrowser.toString()});
return new __nativePromise(function (resolve) {
return __nativePromise.resolve()
.then(_ => ${expression})
.catch(${wrapRuntimeEvalErrorInBrowser.toString()})
.then(resolve);
});
}())`,
includeCommandLineAPI: true,
awaitPromise: true,
Expand Down
257 changes: 0 additions & 257 deletions lighthouse-core/third_party/promise-polyfill/promise.js

This file was deleted.

Loading

0 comments on commit 44a634f

Please sign in to comment.