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

Move AMP runtime to standard fetch #16707

Closed
5 tasks
prateekbh opened this issue Jul 12, 2018 · 2 comments
Closed
5 tasks

Move AMP runtime to standard fetch #16707

prateekbh opened this issue Jul 12, 2018 · 2 comments

Comments

@prateekbh
Copy link
Member

prateekbh commented Jul 12, 2018

This issue serves as a tracker to move AMP to standard fetch instead of the used poly/ponyfills of fetch, FetchResponse and FetchResponseHeaders.

The above methods/classes are always used instead of native fetch if we want to request a document over the network. This effort will isolate this case in a separate ES-module and move all the rest of xhr-impl.js over to native fetch whenever present.

The entire migration will be done in four tasks.

  • Create a separate DocumentFetcher ES module and implement fetchDocument.
  • Shift current definitions of fetch, FetchResponse and FetchResponseHeaders to a fetch-polyfill instead.
  • Move all existing usage to document fetcher and remove all the code for fetchDocument form xhr-impl.js.
  • Rename Services.xhr to Services.fetch.
  • Decide on a uniform method to use .json(), .text() etc methods from fetch and create eslint rules to confirm it throughout the codebase.
@ampprojectbot
Copy link
Member

This issue seems to be in Pending Triage for awhile. @prateekbh Please triage this to an appropriate milestone.

@prateekbh
Copy link
Member Author

Fixed by #17642

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