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

react, react-test-renderer: Update React types for 16.9.0 #37426

Merged
merged 1 commit into from Aug 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 20 additions & 3 deletions types/react-test-renderer/index.d.ts
@@ -1,4 +1,4 @@
// Type definitions for react-test-renderer 16.8
// Type definitions for react-test-renderer 16.9
// Project: https://facebook.github.io/react/
// Definitions by: Arvitaly <https://github.com/arvitaly>
// Lochbrunner <https://github.com/lochbrunner>
Expand Down Expand Up @@ -53,6 +53,20 @@ export interface TestRendererOptions {
}
export function create(nextElement: ReactElement, options?: TestRendererOptions): ReactTestRenderer;

/**
* Wrap any code rendering and triggering updates to your components into `act()` calls.
*
* Ensures that the behavior in your tests matches what happens in the browser
* more closely by executing pending `useEffect`s before returning. This also
* reduces the amount of re-renders done.
*
* @param callback An asynchronous, void callback that will execute as a single, complete React commit.
*
* @see https://reactjs.org/blog/2019/02/06/react-v16.8.0.html#testing-hooks
*/
// the "void | undefined" is here to forbid any sneaky return values
// tslint:disable-next-line: void-return
export function act(callback: () => Promise<void | undefined>): Promise<undefined>;
Copy link
Member Author

Choose a reason for hiding this comment

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

() => Promise<void> would essentially accept Promise<any>, so it's |ed with undefined to enforce no values are returned. Just Promise<undefined> would reject void functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this affect the test renderer runtime at all if the Promise resolves to a defined value? I think some developers like to combine early returns with an evaluation (still not caring about the return value)

if (somethingThatMakesMeReturnEarly) {
  return doSomethingElse()
}

Choose a reason for hiding this comment

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

What's the problem with Promise<any>?

I'm incurring into an error when trying to do something like this:

await act(() =>
  expect(result.current.validate()).rejects.toEqual({ a: "error" })
);

expect(somePromise).rejects.toEqual() will return the same type returned by somePromise. Test passes, but TypeScript is complaining: Type 'Promise<SomePromiseReturn>' is not assignable to type 'void'.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@diegoazh Please open an issue with the full error message. May also be an issue with the jest typings.

In any case: I would keep it simply an just await result.current.validate() toThrow and wrap it in a callback that's just void. It's not obvious anyway that expect.rejects returns a promise which makes this act harder to read than necessary.

/**
* Wrap any code rendering and triggering updates to your components into `act()` calls.
*
Expand All @@ -71,6 +85,9 @@ export function act(callback: () => void | undefined): DebugPromiseLike;
// Intentionally doesn't extend PromiseLike<never>.
// Ideally this should be as hard to accidentally use as possible.
export interface DebugPromiseLike {
// the actual then() in here is 0-ary, but that doesn't count as a PromiseLike.
then(onfulfilled: (value: never) => never, onrejected: (reason: never) => never): never;
// the actual then() in here is 1-ary, but that doesn't count as a PromiseLike.
then(
onfulfilled: (value: never) => never,
onrejected: (reason: never) => never,
): never;
}
15 changes: 13 additions & 2 deletions types/react-test-renderer/react-test-renderer-tests.ts
Expand Up @@ -70,8 +70,19 @@ shallowRenderer.getMountedInstance();
// Only synchronous, void callbacks are acceptable for act()
act(() => {});
// $ExpectError
act(async () => {});
// $ExpectError
act(() => null);
// $ExpectError
Promise.resolve(act(() => {}));

// async act is now acceptable in React 16.9,
// but the result must be void or undefined
Promise.resolve(act(async () => {}));

void (async () => {
act(() => {});

await act(async () => {});
await act(async () => undefined);
// $ExpectError
await act(async () => null);
})();
12 changes: 3 additions & 9 deletions types/react/index.d.ts
@@ -1,4 +1,4 @@
// Type definitions for React 16.8
// Type definitions for React 16.9
// Project: http://facebook.github.io/react/
// Definitions by: Asana <https://asana.com>
// AssureSign <http://www.assuresign.com>
Expand Down Expand Up @@ -349,13 +349,6 @@ declare namespace React {

/** A fallback react tree to show when a Suspense child (like React.lazy) suspends */
fallback: NonNullable<ReactNode>|null;

// I tried looking at the code but I have no idea what it does.
// https://github.com/facebook/react/issues/13206#issuecomment-432489986
/**
* Not implemented yet, requires unstable_ConcurrentMode
*/
// maxDuration?: number;
Copy link
Member Author

Choose a reason for hiding this comment

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

Both this and unstable_ConcurrentMode were removed. unstable_ConcurrentMode was never added to the types anyway.

}
/**
* This feature is not yet available for server-side rendering.
Expand All @@ -382,7 +375,7 @@ declare namespace React {
onRender: ProfilerOnRenderCallback;
}

const unstable_Profiler: ExoticComponent<ProfilerProps>;
const Profiler: ExoticComponent<ProfilerProps>;
Copy link
Member Author

Choose a reason for hiding this comment

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

Just renamed.


//
// Component API
Expand Down Expand Up @@ -2199,6 +2192,7 @@ declare namespace React {
playsInline?: boolean;
poster?: string;
width?: number | string;
disablePictureInPicture?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

}

// this list is "complete" in that it contains every SVG attribute
Expand Down
12 changes: 9 additions & 3 deletions types/react/test/tsx.tsx
Expand Up @@ -321,7 +321,7 @@ const ForwardRef3 = React.forwardRef(
<ForwardRef3 ref={divFnRef}/>;
<ForwardRef3 ref={divRef}/>;

const Profiler = React.unstable_Profiler;
const { Profiler } = React;

// 'id' is missing
<Profiler />; // $ExpectError
Expand Down Expand Up @@ -357,8 +357,14 @@ const Profiler = React.unstable_Profiler;
</Profiler>;

type ImgProps = React.ComponentProps<'img'>;
// $ExpectType "async" | "auto" | "sync" | undefined
type ImgPropsDecoding = ImgProps['decoding'];
const imgProps: ImgProps = {};
// the order of the strings in the union seems to vary
// with the typescript version, so test assignment instead
Copy link
Member Author

Choose a reason for hiding this comment

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

Sometimes it's "async" | "auto" | "sync" | undefined, sometimes it's "auto" | "async" | "sync" | undefined. Not sure why. It doesn't matter for type checking, but dtslint cares about it.

imgProps.decoding = 'async';
imgProps.decoding = 'auto';
imgProps.decoding = 'sync';
// $ExpectError
imgProps.decoding = 'nonsense';
type ImgPropsWithRef = React.ComponentPropsWithRef<'img'>;
// $ExpectType ((instance: HTMLImageElement | null) => void) | RefObject<HTMLImageElement> | null | undefined
type ImgPropsWithRefRef = ImgPropsWithRef['ref'];
Expand Down