Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Runtime.evaluate errors on polyfilled Promises #1173

Closed
brendankenny opened this issue Dec 17, 2016 · 5 comments
Closed

Regression: Runtime.evaluate errors on polyfilled Promises #1173

brendankenny opened this issue Dec 17, 2016 · 5 comments
Assignees

Comments

@brendankenny
Copy link
Member

Getting Protocol error (Runtime.evaluate): Result of the evaluation is not a promise for all driver.evaluateAsync calls on test angular site like e.g. https://material2-app.firebaseapp.com/

#1037 handles this case (and we've tested it pretty thoroughly in the past), so need to investigate how this regressed.

@patrickhulce
Copy link
Collaborator

:(

@patrickhulce patrickhulce self-assigned this Dec 19, 2016
@patrickhulce
Copy link
Collaborator

Found it. Looks like however Angular2 polyfills promise goes above and beyond what the smokehouse promise polyfill does and removing the new __nativePromise layer during feedback broke it for angular but not smokehouse. In a nutshell, there doesn't seem to be a way around explicitly constructing a new promise since returning a polyfilled Promise within a native Promise .then doesn't always work, just when you're testing ;)

@brendankenny
Copy link
Member Author

brendankenny commented Dec 19, 2016

looks like the problem was that zone.js not only overwrites the global Promise, it also overwrites Promise.prototype.then on the original global.

So even though we were using our window.__nativePromise, we were getting the overwritten window.__nativePromise.prototype.then, which returns a non-native promise :(

@brendankenny
Copy link
Member Author

fixed in #1178

@paulirish
Copy link
Member

some history from #11190 ...

Lighthouse backstory:
Zone aggressively polyfills promises including rewriting Promise.prototype.then which caused
problems for our evaluateAsync code where we rely on a Promise to work.
We serve this file in dbw_tester.html to make sure evaluateAsync, etc all works great
Serving this file via node_modules was a pain so we've replaced it with our own.
@see #1173
@see #1178
We were serving zone.js@0.7.3 from node_modules but that introduced complexity, so it was
vendored into a third_party folder.
@see #9672
Later we decided to stop using this fancy file altogether so we can just ignore drama from both
third_party placement and node_modules imports.
@see #11043
This promise polyfill isn't as aggressive as zone as it doesn't patch every interface
that returns a promise (https://github.com/angular/zone.js/blob/v0.7.3/dist/zone.js#L1589-L1611)
But we think that's ok.

but 11190 was a pain to land and the earlier annoyances calmed down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants