Skip to content

Initialize JavaScript tests with polyfilled fetch #7518

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.

Sign up for GitHub

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

Merged
aduth merged 1 commit into from
Dec 20, 2022
Merged

Initialize JavaScript tests with polyfilled fetch #7518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions app/javascript/packages/document-capture/components/address-search.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { render } from '@testing-library/react';
import { setupServer } from 'msw/node';
import { rest } from 'msw';
import type { SetupServerApi } from 'msw/node';
import { fetch } from 'whatwg-fetch';
import { useSandbox } from '@18f/identity-test-helpers';
import userEvent from '@testing-library/user-event';
import AddressSearch, { ADDRESS_SEARCH_URL } from './address-search';
Expand All @@ -26,7 +25,6 @@ describe('AddressSearch', () => {

let server: SetupServerApi;
before(() => {
global.window.fetch = fetch;
server = setupServer(
rest.post(ADDRESS_SEARCH_URL, (_req, res, ctx) => res(ctx.json(DEFAULT_RESPONSE))),
);
Expand All @@ -35,7 +33,6 @@ describe('AddressSearch', () => {

after(() => {
server.close();
global.window.fetch = () => Promise.reject(new Error('Fetch must be stubbed'));
});

it('fires the callback with correct input', async () => {
Expand Down
4 changes: 3 additions & 1 deletion app/javascript/packs/form-steps-wait.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ interface FormStepsWaitElements {
form: HTMLFormElement;
}

type FetchOrFetchPolyfill = typeof window.fetch & { polyfill?: boolean };
Copy link
Copy Markdown
Contributor

@allthesignals allthesignals Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jw: what is this syntax? merge of the surface of window.fetch with an additional flag?

Copy link
Copy Markdown
Contributor Author

@aduth aduth Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


interface FormStepsWaitOptions {
/**
* Poll interval.
Expand Down Expand Up @@ -118,7 +120,7 @@ export class FormStepsWait {
body: new window.FormData(form),
});

if ('polyfill' in window.fetch) {
if ((window.fetch as FetchOrFetchPolyfill).polyfill) {
Copy link
Copy Markdown
Contributor Author

@aduth aduth Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change from in to truthy check, since in will pass for the stubbed value of undefined.

The polyfill sets it to true (source).

Copy link
Copy Markdown
Contributor

@allthesignals allthesignals Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh interesting. so if someone had polyfill: false it'd still pass.

Copy link
Copy Markdown
Contributor Author

@aduth aduth Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// The fetch polyfill is implemented using XMLHttpRequest, which suffers from an issue where a
// Content-Type header from a POST is carried into a redirected GET, which is exactly the flow
// we are handling here. The current version of Rack neither handles nor provides easy insight
Expand Down
4 changes: 4 additions & 0 deletions spec/javascripts/packs/form-steps-wait-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ describe('FormStepsWait', () => {
return form;
}

beforeEach(() => {
sandbox.stub(window.fetch, 'polyfill').value(undefined);
});

it('submits form via fetch', () => {
const action = new URL('/', window.location).toString();
const method = 'post';
Expand Down
4 changes: 2 additions & 2 deletions spec/javascripts/spec_helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import chai from 'chai';
import dirtyChai from 'dirty-chai';
import sinonChai from 'sinon-chai';
import chaiAsPromised from 'chai-as-promised';
import { Response } from 'whatwg-fetch';
import { fetch, Response } from 'whatwg-fetch'; // Remove in favor of native fetch in Node v20+ (https://nodejs.org/docs/latest/api/globals.html#fetch)
import { createDOM, useCleanDOM } from './support/dom';
import { chaiConsoleSpy, useConsoleLogSpy } from './support/console';
import { sinonChaiAsPromised } from './support/sinon';
Expand All @@ -26,7 +26,7 @@ const windowGlobals = Object.fromEntries(
.map((key) => [key, window[key]]),
);
Object.assign(global, windowGlobals);
global.window.fetch = () => Promise.reject(new Error('Fetch must be stubbed'));
global.window.fetch = fetch;
Object.defineProperty(global.window, 'crypto', { value: webcrypto });
global.window.URL.createObjectURL = createObjectURLAsDataURL;
global.window.URL.revokeObjectURL = () => {};
Expand Down