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

Avoid calling Object.assign before polyfill is installed #17544

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

dreamofabear
Copy link

@dreamofabear dreamofabear commented Aug 16, 2018

Breaks AMP on browsers that don't have Object.assign which appears to be Chrome 44 and earlier among others. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Browser_compatibility

Verified locally that this fixes the bug in Chrome 41.

Context: b/112598137

/to @jridgewell @rsimha

Fixes #17505
Fixes #17533
Fixes #17528
Fixes #17547

@dreamofabear dreamofabear added the Externally Tracked Tracked by Google or other parties label Aug 16, 2018
@rsimha
Copy link
Contributor

rsimha commented Aug 16, 2018

Is it feasible to write a unit test (or an error message) for this, such that it would work on latest Chrome (by checking that a polyfilled function is never called before it is installed?)

@dreamofabear
Copy link
Author

Was just thinking about this. We know to import polyfills.js first in entry points, but this bug was due to order of execution in the file itself. To catch that, we can test by importing polyfills.js in an environment that's missing all of the native APIs and making sure there are no runtime errors.

@rsimha
Copy link
Contributor

rsimha commented Aug 16, 2018

The offending code was introduced #17205 In 2246723, which was released last week. I wonder why things only broke one week later.

Edit: It turns out that the release went to canary 6 days ago, but hit prod yesterday. This now make sense.

@rsimha rsimha merged commit 16fbfca into ampproject:master Aug 16, 2018
@dreamofabear dreamofabear deleted the fix-object-assign-bug branch August 16, 2018 17:44
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants