Skip to content

Commit

Permalink
[fei5235.1.ssradapter] Add SSR adapter for test harness (#1991)
Browse files Browse the repository at this point in the history
## Summary:
This adds a new `ssr` adapter to the Wonder Blocks Testing package. It ensures a `RenderStateRoot` component is rendered around the component under test.

This adapter is default off since it may add the need for additional `act` or `waitFor` calls when using Testing Library due to the state changes that `RenderStateRoot` will perform.

In addition, default off ensures that updating to this new version won't break tests that were rendering their own `RenderStateRoot` component; nested `RenderStateRoot` components throw an error by default.

This change also addresses the TypeScript error suppressions in Wonder Blocks Testing as well as one in Wonder Blocks Core.

I also added a `yarn typewatch` command to package.json. This is a useful command to run in a separate terminal while developing to ensure that your changes don't break the type definitions, saves a few keystrokes, and is easier to remember/discover.

Issue: FEI-5235

## Test plan:
`yarn jest`
`yarn typecheck`

Author: somewhatabstract

Reviewers: jeresig

Required Reviewers:

Approved By: jeresig

Checks: ⏭  dependabot, ⌛ gerald, ⌛ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⌛ Chromatic - Build on review PR (push) / chromatic (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on dependabot PRs (push)

Pull Request URL: #1991
  • Loading branch information
somewhatabstract authored Aug 22, 2023
1 parent 65c02cf commit 1920feb
Show file tree
Hide file tree
Showing 29 changed files with 202 additions and 105 deletions.
5 changes: 5 additions & 0 deletions .changeset/blue-ways-cough.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-testing": major
---

Added new SSR adapter for test harnesses to support `RenderStateRoot` in tests and stories
5 changes: 5 additions & 0 deletions .changeset/large-paws-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-core": patch
---

`RenderStateRoot` now wraps children in a React fragment
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const DefaultAdapters = {
data: data.adapter,
portal: portal.adapter,
router: router.adapter,
ssr: ssr.adapter,
};

/**
Expand All @@ -30,6 +31,7 @@ const DefaultConfigs: Configs<typeof DefaultAdapters> = {
data: data.defaultConfig,
portal: portal.defaultConfig,
router: router.defaultConfig,
ssr: ssr.defaultConfig,
};
```

Expand All @@ -43,6 +45,7 @@ There are four build-in adapters:
- [`data`](#data)
- [`portal`](#portal)
- [`router`](#router)
- [`ssr`](#ssr)

## css

Expand Down Expand Up @@ -186,3 +189,13 @@ There are four distinct shapes to the configuration:
The first of these provides the most flexibility, basically supporting full configuration of a `MemoryRouter`, which supports history and navigation. The second configuration type is for scenarios where we absolutely do not want navigation, such as server-side rendering scenarios; it forces the use of a `StaticRouter` instance. The third type is for when you just need to render a single location with a matched route path. The fourth and final type allows you to just provide the location as a string.

By default, the router adapter is configured with `{location: "/"}`.

## ssr

The `ssr` adapter ensures that the render state is being managed by rendering a `RenderStateRoot` component around the harnessed component.

```ts
type Config = true | null;
```

By default, this adapter is off since state changes occur when `RenderStateRoot` is used that may require tests to make accommodations, such as adding appropriate `act` or `waitFor` calls when using Testing Library.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const DefaultAdapters = {
data: data.adapter,
portal: portal.adapter,
router: router.adapter,
ssr: ssr.adapter,
};
```

Expand All @@ -40,6 +41,7 @@ type DefaultAdaptersType = {|
data: typeof data.adapter,
portal: typeof portal.adapter,
router: typeof router.adapter,
ssr: typeof ssr.adapter,
|};
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,19 @@ const DefaultAdapters = {
data: data.adapter,
portal: portal.adapter,
router: router.adapter,
ssr: ssr.adapter,
};
```

`DefaultAdapters` is not strongly typed to `TestHarnessAdapters`. Instead, its type is:

```ts
type DefaultAdaptersType = {|
css: typeof css.adapter,
data: typeof data.adapter,
portal: typeof portal.adapter,
router: typeof router.adapter,
css: typeof css.adapter,
data: typeof data.adapter,
portal: typeof portal.adapter,
router: typeof router.adapter,
ssr: typeof ssr.adapter,
|};
```

Expand All @@ -49,6 +51,7 @@ const DefaultConfigs: TestHarnessConfigs<typeof DefaultAdapters> = {
data: data.defaultConfig,
portal: portal.defaultConfig,
router: router.defaultConfig,
ssr: ssr.defaultConfig,
};
```

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"test:coverage": "yarn run test:common && yarn run jest --coverage",
"test": "yarn run test:common && yarn run jest",
"typecheck": "tsc",
"typewatch": "tsc --noEmit --watch --incremental",
"add:devdepbysha": "bash -c 'yarn add -W --dev \"git+https://[email protected]/Khan/$0.git#$1\"'"
},
"author": "",
Expand Down Expand Up @@ -136,4 +137,4 @@
"@types/react": "16",
"@types/react-dom": "16"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ const RenderStateRoot = ({
}
// Avoid rendering multiple providers if this RenderStateRoot
// is nested inside another one.
// @ts-expect-error [FEI-5019] - TS2322 - Type 'ReactNode' is not assignable to type 'ReactElement<any, string | JSXElementConstructor<any>>'.
return children;
return <>{children}</>;
}

const value = firstRender ? RenderState.Initial : RenderState.Standard;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ const getHref = (input: RequestInfo): string => {
return input;
} else if (typeof input.url === "string") {
return input.url;
// @ts-expect-error [FEI-5019] - TS2339 - Property 'href' does not exist on type 'Request'.
} else if (typeof input.href === "string") {
// @ts-expect-error [FEI-5019] - TS2339 - Property 'href' does not exist on type 'Request'.
return input.href;
} else if (typeof (input as any).href === "string") {
return (input as any).href;
} else {
throw new Error(`Unsupported input type`);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/wonder-blocks-testing/src/fetch/mock-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {FetchMockFn, FetchMockOperation} from "./types";
* A mock for the fetch function passed to GqlRouter.
*/
export const mockFetch = (): FetchMockFn =>
mockRequester<FetchMockOperation, any>(
mockRequester<FetchMockOperation>(
fetchRequestMatchesMock,
// NOTE(somewhatabstract): The indentation is expected on the lines
// here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,9 @@ describe("fixtures", () => {

it("should render the component", () => {
// Arrange
const fixture = fixtures(
// @ts-expect-error [FEI-5019] - TS2345 - Argument of type '(props: any) => string' is not assignable to parameter of type 'ComponentType<any>'.
(props: any) => `I rendered ${JSON.stringify(props)}`,
);
const fixture = fixtures((props: any) => (
<>{`I rendered ${JSON.stringify(props)}`}</>
));
const Fixture: any = fixture("A simple story", {});

// Act
Expand All @@ -131,16 +130,12 @@ describe("fixtures", () => {

it("should render the wrapper", () => {
// Arrange
const fixture = fixtures(
// @ts-expect-error [FEI-5019] - TS2345 - Argument of type '(props: any) => string' is not assignable to parameter of type 'ComponentType<any>'.
(props: any) => `I rendered ${JSON.stringify(props)}`,
);
const Fixture: any = fixture(
"A simple story",
{},
// @ts-expect-error [FEI-5019] - TS2345 - Argument of type '() => string' is not assignable to parameter of type 'ComponentType<any> | undefined'.
() => "I am a wrapper",
);
const fixture = fixtures((props: any) => (
<>{`I rendered ${JSON.stringify(props)}`}</>
));
const Fixture: any = fixture("A simple story", {}, () => (
<>I am a wrapper</>
));

// Act
render(<Fixture aProp="aValue" />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@ type Props = {
propB?: string;
};

const MyComponent = (props: Props): React.ReactElement =>
// @ts-expect-error: `string` is not a valid `ReactElement`.
`I am a component. Here are my props: ${JSON.stringify(props, null, 2)}`;
const MyComponent = (props: Props): React.ReactElement => (
<>
{`I am a component. Here are my props: ${JSON.stringify(
props,
null,
2,
)}`}
</>
);

const Wrapper = (props: any) => (
<>
Expand Down
3 changes: 1 addition & 2 deletions packages/wonder-blocks-testing/src/fixtures/fixtures.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ export const fixtures = <
let storyNumber = 1;

const getPropsOptions = {
// @ts-expect-error [FEI-5019] - TS7006 - Parameter 'message' implicitly has an 'any' type. | TS7019 - Rest parameter 'args' implicitly has an 'any[]' type.
log: (message, ...args) => action(message)(...args),
log: (message: string, ...args: Array<any>) => action(message)(...args),
logHandler: action,
} as const;

Expand Down
2 changes: 1 addition & 1 deletion packages/wonder-blocks-testing/src/gql/mock-gql-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {GqlFetchMockFn, GqlMockOperation} from "./types";
* A mock for the fetch function passed to GqlRouter.
*/
export const mockGqlFetch = (): GqlFetchMockFn =>
mockRequester<GqlMockOperation<any, any, any>, any>(
mockRequester<GqlMockOperation<any, any, any>>(
gqlRequestMatchesMock,
// Note that the identation at the start of each line is important.
// TODO(somewhatabstract): Make a stringify that indents each line of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ describe("#hookHarness", () => {
const config = {
router: "/boo",
} as const;
// @ts-expect-error [FEI-5019] - TS2339 - Property 'harnessFake' does not exist on type 'typeof import("/Users/kevinbarabash/khan/wonder-blocks/packages/wonder-blocks-testing/src/harness/make-hook-harness")'.
// @ts-expect-error We know harnessFake isn't real, we add it in the
// mocks at the top of this file.
const [{harnessFake}, {hookHarness}] = await ws.isolateModules(() =>
Promise.all([
import("../make-hook-harness"),
Expand All @@ -54,7 +55,8 @@ describe("#hookHarness", () => {
const config = {
router: "/boo",
} as const;
// @ts-expect-error [FEI-5019] - TS2339 - Property 'returnValueFake' does not exist on type 'typeof import("/Users/kevinbarabash/khan/wonder-blocks/packages/wonder-blocks-testing/src/harness/make-hook-harness")'.
// @ts-expect-error We know harnessFake isn't real, we add it in the
// mocks at the top of this file.
const [{returnValueFake}, {hookHarness}] = await ws.isolateModules(() =>
Promise.all([
import("../make-hook-harness"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,21 @@ import {renderAdapters} from "../render-adapters";
import type {TestHarnessAdapter, TestHarnessConfigs} from "../types";

describe("#renderAdapters", () => {
it("should return children if no adapters", () => {
it("should render children if no adapters", () => {
// Arrange
const children = <div>Adapt me!</div>;

// Act
const result = renderAdapters({}, {}, children);

// Assert
expect(result).toBe(children);
expect(result).toMatchInlineSnapshot(`
<React.Fragment>
<div>
Adapt me!
</div>
</React.Fragment>
`);
});

it("should invoke the adapter with its corresponding config", () => {
Expand All @@ -38,10 +44,8 @@ describe("#renderAdapters", () => {
it("should render each adapter and the children", () => {
// Arrange
const children = "Adapt me!";
// @ts-expect-error: `string` is not a valid `ReactElement`.
const adapter: TestHarnessAdapter<string> = (c: any, conf: any) => {
return `${conf}:${c}`;
};
const adapter: TestHarnessAdapter<string> = (c: any, conf: any) =>
`${conf}:${c}` as any;
const adapters = {
adapterA: adapter,
adapterB: adapter,
Expand All @@ -57,16 +61,18 @@ describe("#renderAdapters", () => {
const result = renderAdapters(adapters, configs, children);

// Assert
expect(result).toMatchInlineSnapshot(`"C:B:A:Adapt me!"`);
expect(result).toMatchInlineSnapshot(`
<React.Fragment>
C:B:A:Adapt me!
</React.Fragment>
`);
});

it("should skip adapters where the corresponding config is null", () => {
// Arrange
const children = "Adapt me!";
// @ts-expect-error: `string` is not a valid `ReactElement`.
const adapter: TestHarnessAdapter<string> = (c: any, conf: any) => {
return `${conf}:${c}`;
};
const adapter: TestHarnessAdapter<string> = (c: any, conf: any) =>
`${conf}:${c}` as any;
const adapters = {
adapterA: adapter,
adapterB: adapter,
Expand All @@ -82,6 +88,10 @@ describe("#renderAdapters", () => {
const result = renderAdapters(adapters, configs, children);

// Assert
expect(result).toMatchInlineSnapshot(`"C:A:Adapt me!"`);
expect(result).toMatchInlineSnapshot(`
<React.Fragment>
C:A:Adapt me!
</React.Fragment>
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ describe("#testHarness", () => {
const config = {
router: "/boo",
} as const;
// @ts-expect-error [FEI-5019] - TS2339 - Property 'harnessFake' does not exist on type 'typeof import("/Users/kevinbarabash/khan/wonder-blocks/packages/wonder-blocks-testing/src/harness/make-test-harness")'.
// @ts-expect-error We know harnessFake isn't real, we add it in the
// mocks at the top of this file.
const [{harnessFake}, {testHarness}] = await ws.isolateModules(() =>
Promise.all([
import("../make-test-harness"),
Expand All @@ -56,7 +57,8 @@ describe("#testHarness", () => {
const config = {
router: "/boo",
} as const;
// @ts-expect-error [FEI-5019] - TS2339 - Property 'returnValueFake' does not exist on type 'typeof import("/Users/kevinbarabash/khan/wonder-blocks/packages/wonder-blocks-testing/src/harness/make-test-harness")'.
// @ts-expect-error We know harnessFake isn't real, we add it in the
// mocks at the top of this file.
const [{returnValueFake}, {testHarness}] = await ws.isolateModules(() =>
Promise.all([
import("../make-test-harness"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@ import type {
*/

//> should assert type of config.
// @ts-expect-error [FEI-5019] - TS2352 - Conversion of type '(children: React.ReactNode, config: number) => React.ReactElement<any>' to type 'TestHarnessAdapter<string>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
((
children: React.ReactNode,
// TConfig is string, but we typed this arg as a number
// $FlowExpectedError[incompatible-cast]
config: number,
): React.ReactElement<any> => <div />) as TestHarnessAdapter<string>;
// @ts-expect-error TConfig is string, but we typed config as a number
((children: React.ReactNode, config: number): React.ReactElement<any> => (
<div />
)) as TestHarnessAdapter<string>;
//<

//> should work for correct definition
Expand All @@ -36,10 +33,8 @@ import type {
//<

//> should assert if adapter is not Adapter<TConfig>
// @ts-expect-error [FEI-5019] - TS2352 - Conversion of type '{ adapterString: string; }' to type 'TestHarnessAdapters' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
// @ts-expect-error String is not a adapter function
({
// String is not a adapter function
// $FlowExpectedError[incompatible-cast]
adapterString: "string",
} as TestHarnessAdapters);
//<
Expand Down Expand Up @@ -100,11 +95,9 @@ const adapters = {
//<

//> should assert if config does not match adapter config
// @ts-expect-error: Conversion of type '{ adapterA: string; adapterB: string; }' to type 'TestHarnessConfigs<{ readonly adapterA: TestHarnessAdapter<string>; readonly adapterB: TestHarnessAdapter<number>; }>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first. Types of property 'adapterB' are incompatible. Type 'string' is not comparable to type 'number'.
// @ts-expect-error: the config type here is a number, not a string
({
adapterA: "a string, this is correct",
// the config type here is a number, not a string
// $FlowExpectedError[incompatible-cast]
adapterB: "a string, but it should be a number",
} as TestHarnessConfigs<typeof adapters>);
//<
Loading

0 comments on commit 1920feb

Please sign in to comment.