From a8aaac33921f31c65e8cb02c0eb66d15fc4f019e Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Thu, 1 Aug 2024 15:54:40 -0700 Subject: [PATCH] ImageLoader: fix types and remove @ts-expect-error instances (#1474) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: While chasing down a question from LearningEquality, I noticed some `@ts-expect-error` usages that made me wonder. After removing them I found it was quite easy to make the types correct. This is also a common problem in our codebase following the port from Flow to TypeScript. We often set props to have the type `SomeType | null | undefined`. However, often we don't need to support it being `null`. In this case, removing the assignment of a default value (`null`) fixed the main issue. Issue: "none" ## Test plan: `yarn typecheck` `yarn lint` Author: jeremywiebe Reviewers: jeremywiebe, mark-fitzgerald, benchristel, SonicScrewdriver, nishasy, catandthemachines, anakaren-rojas Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1474 --- .changeset/blue-moons-carry.md | 5 +++++ .../components/__stories__/image-loader.stories.tsx | 3 --- packages/perseus/src/components/image-loader.tsx | 12 ++++-------- packages/perseus/src/components/svg-image.tsx | 4 ---- packages/perseus/src/server-item-renderer.tsx | 1 - 5 files changed, 9 insertions(+), 16 deletions(-) create mode 100644 .changeset/blue-moons-carry.md diff --git a/.changeset/blue-moons-carry.md b/.changeset/blue-moons-carry.md new file mode 100644 index 0000000000..410005d1e2 --- /dev/null +++ b/.changeset/blue-moons-carry.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Minor type fixes in the `ImageLoader` and `SvgImage` components diff --git a/packages/perseus/src/components/__stories__/image-loader.stories.tsx b/packages/perseus/src/components/__stories__/image-loader.stories.tsx index 5896950b5b..08abacefea 100644 --- a/packages/perseus/src/components/__stories__/image-loader.stories.tsx +++ b/packages/perseus/src/components/__stories__/image-loader.stories.tsx @@ -20,7 +20,6 @@ export const SvgImage = (args: StoryArgs): React.ReactElement => { return ( ReactElement> | null | undefined'. preloader={null} imgProps={{ alt: "ALT", @@ -34,7 +33,6 @@ export const PngImage = (args: StoryArgs): React.ReactElement => { return ( ReactElement> | null | undefined'. preloader={null} imgProps={{ alt: "ALT", @@ -50,7 +48,6 @@ export const InvalidImageWithChildrenForFailedLoading = ( return ( ReactElement> | null | undefined'. preloader={null} imgProps={{ alt: "ALT", diff --git a/packages/perseus/src/components/image-loader.tsx b/packages/perseus/src/components/image-loader.tsx index a9c3e7094b..18f7de644b 100644 --- a/packages/perseus/src/components/image-loader.tsx +++ b/packages/perseus/src/components/image-loader.tsx @@ -28,7 +28,7 @@ export type ImageProps = { alt: string; title?: string; ["aria-hidden"]?: boolean; - tabIndex?: string; + tabIndex?: number; onClick?: (e: React.SyntheticEvent) => void; style?: Dimensions; }; @@ -41,7 +41,7 @@ type Props = { // When the DOM updates to replace the preloader with the image, or // vice-versa, we trigger this callback. onUpdate: (status: (typeof Status)[keyof typeof Status]) => void; - preloader: () => React.ReactElement | null | undefined; + preloader: (() => React.ReactNode) | null | undefined; src: string; }; @@ -122,17 +122,15 @@ class ImageLoader extends React.Component { renderImg: () => React.ReactElement> = () => { const {src, imgProps} = this.props; - let onKeyUp = null; - let onKeyDown = null; + let onKeyUp; + let onKeyDown; if (imgProps.onClick != null) { - // @ts-expect-error - TS2322 - Type '(e: React.KeyboardEvent) => void' is not assignable to type 'null'. onKeyUp = (e: React.KeyboardEvent) => { // 13 is enter key, 32 is space key if (e.keyCode === 13 || e.keyCode === 32) { imgProps.onClick && imgProps.onClick(e); } }; - // @ts-expect-error - TS2322 - Type '(e: React.KeyboardEvent) => void' is not assignable to type 'null'. onKeyDown = (e: React.KeyboardEvent) => { // 32 is space key if (e.keyCode === 32) { @@ -146,9 +144,7 @@ class ImageLoader extends React.Component { return ( | undefined'. onKeyUp={onKeyUp} - // @ts-expect-error - TS2322 - Type 'null' is not assignable to type 'KeyboardEventHandler | undefined'. onKeyDown={onKeyDown} {...imgProps} /> diff --git a/packages/perseus/src/components/svg-image.tsx b/packages/perseus/src/components/svg-image.tsx index 40093e2806..f98ad84730 100644 --- a/packages/perseus/src/components/svg-image.tsx +++ b/packages/perseus/src/components/svg-image.tsx @@ -667,7 +667,6 @@ class SvgImage extends React.Component { Element) | null' is not assignable to type '() => ReactElement> | null | undefined'. preloader={preloader} onUpdate={this.handleUpdate} /> @@ -679,7 +678,6 @@ class SvgImage extends React.Component { return ( Element) | null' is not assignable to type '() => ReactElement> | null | undefined'. preloader={preloader} imgProps={imageProps} onUpdate={this.handleUpdate} @@ -742,7 +740,6 @@ class SvgImage extends React.Component { src={imageUrl} onLoad={this.onImageLoad} onUpdate={this.handleUpdate} - // @ts-expect-error - TS2322 - Type '(() => Element) | null' is not assignable to type '() => ReactElement> | null | undefined'. preloader={preloader} imgProps={imageProps} /> @@ -758,7 +755,6 @@ class SvgImage extends React.Component { src={imageUrl} onLoad={this.onImageLoad} onUpdate={this.handleUpdate} - // @ts-expect-error - TS2322 - Type '(() => Element) | null' is not assignable to type '() => ReactElement> | null | undefined'. preloader={preloader} imgProps={imageProps} /> diff --git a/packages/perseus/src/server-item-renderer.tsx b/packages/perseus/src/server-item-renderer.tsx index ec6889ec23..2a7f56024f 100644 --- a/packages/perseus/src/server-item-renderer.tsx +++ b/packages/perseus/src/server-item-renderer.tsx @@ -72,7 +72,6 @@ type SerializedState = { hints: any; }; -/* eslint-disable-next-line react/no-unsafe */ export class ServerItemRenderer extends React.Component implements RendererInterface, KeypadContextRendererInterface