Initialize JavaScript tests with polyfilled fetch#7518
Conversation
changelog: Internal, Automated Tests, Improve JavaScript tests simulating network requests
| }); | ||
|
|
||
| if ('polyfill' in window.fetch) { | ||
| if ((window.fetch as FetchOrFetchPolyfill).polyfill) { |
There was a problem hiding this comment.
Change from in to truthy check, since in will pass for the stubbed value of undefined.
The polyfill sets it to true (source).
There was a problem hiding this comment.
oh interesting. so if someone had polyfill: false it'd still pass.
There was a problem hiding this comment.
Yeah, the only way it could work previously is if we would delete the property altogether (delete fetch.polyfill), but there's not a clean with to stub that.
allthesignals
left a comment
allthesignals
left a comment
There was a problem hiding this comment.
Awesome lgtm!
Change defaults test suite to native fetch (rather than defaulting to requiring a polyfill), making things future-friendly.
| form: HTMLFormElement; | ||
| } | ||
|
|
||
| type FetchOrFetchPolyfill = typeof window.fetch & { polyfill?: boolean }; |
There was a problem hiding this comment.
Jw: what is this syntax? merge of the surface of window.fetch with an additional flag?
There was a problem hiding this comment.
Yeah, without this, TypeScript will become upset about trying to reference fetch.polyfill, since polyfill is not a property on the default fetch implementation, only in the polyfill.
So we typecast fetch to a type which effectively extends the default fetch type with the additional polyfill property.
Specifically, the syntax is an intersection type: https://www.typescriptlang.org/docs/handbook/2/objects.html#intersection-types
| }); | ||
|
|
||
| if ('polyfill' in window.fetch) { | ||
| if ((window.fetch as FetchOrFetchPolyfill).polyfill) { |
There was a problem hiding this comment.
oh interesting. so if someone had polyfill: false it'd still pass.
2 participants
🛠 Summary of changes
Replaces default spec behavior of throwing on attempted
fetchrequest with working polyfill.This was originally planned for #7502, but blocked by failures in
form-steps-wait-spec.tsx, addressed here.The issue with the specs is that
form-steps-wait.tsxhas special handling for polyfill'd fetch, due to circumstances of real browsers handling ofXMLHttpRequest. Since we are expecting to fully simulate the network request, we should bypass this workaround handling. (Aside: The workaround will be removed anyways as part of upcoming sunsetting of Internet Explorer support)📜 Testing Plan