[APP-917] Automated typescript check by brick-green · Pull Request #3540 · CDCgov/NEDSS-Modernization

brick-green · 2026-08-04T21:00:16Z

Description

This PR addresses several existing TypeScript errors across component, test, and configuration files. In addition, it establishes a dedicated type-checking step in the Verify Modernization UI workflow to ensure type safety across the development lifecycle.

To run type checking: npm run typecheck

Tickets

Jira Ticket

Checklist before requesting a review

PR focuses on a single story
Code has been fully tested to meet acceptance criteria
PR is reasonably small and reviewable (Generally less than 10 files and 500 changed lines)
All new functions/classes/components reasonably small
Functions/classes/components focused on one responsibility
Code easy to understand and modify (clarity over concise/clever)
PRs containing TypeScript follow the Do's and Don'ts
PR does not contain hardcoded values (Uses constants)
All code is covered by unit or feature tests

…toHaveBeenLastCalledWith

# Conflicts: # apps/modernization-ui/src/apps/deduplication/patient-merge/details/merge-preview/components/administrative-comments/AdministrativeComments.tsx # apps/modernization-ui/src/apps/deduplication/patient-merge/details/merge-preview/components/ethnicity/PreviewEthnicity.tsx # apps/modernization-ui/src/apps/deduplication/patient-merge/details/merge-preview/components/mortality/PreviewMortality.tsx # apps/modernization-ui/src/apps/deduplication/patient-merge/details/merge-preview/components/sex-and-birth/PreviewSexAndBirth.tsx # apps/modernization-ui/src/apps/page-builder/components/DeleteQuestion/DeleteQuestion.tsx # apps/modernization-ui/src/apps/page-builder/components/SaveTemplate/SaveTemplate.spec.tsx # apps/modernization-ui/src/apps/page-builder/page/management/edit/tabs/AddEditTab/AddEditTab.spec.tsx # apps/modernization-ui/src/apps/report/run/filters/advanced/validator.spec.ts # apps/modernization-ui/src/components/MediaBlockBody/MediaBlockBody.tsx # apps/modernization-ui/src/components/TabNavigation/TabNavigation.stories.tsx # apps/modernization-ui/src/components/TopBanner/Banner/banner.tsx # apps/modernization-ui/src/components/TopBanner/BannerButton/BannerButton.tsx # apps/modernization-ui/src/components/TopBanner/BannerContent/BannerContent.tsx # apps/modernization-ui/src/components/TopBanner/BannerFlag/BannerFlag.tsx # apps/modernization-ui/src/components/TopBanner/BannerGuidance/BannerGuidance.tsx # apps/modernization-ui/src/components/TopBanner/BannerHeader/BannerHeader.tsx # apps/modernization-ui/src/components/TopBanner/BannerIcon/BannerIcon.tsx # apps/modernization-ui/src/components/TopBanner/TopBanner.tsx # apps/modernization-ui/src/design-system/card/section/Section.stories.tsx

mcmcgrath13 · 2026-08-10T12:46:46Z

                    onBlur={onBlur}
                    onChange={onChange}
-                    value={value}
+                    value={value ?? undefined}


(nit, b): using undefined as the default here could cause the input to go from uncontrolled to controlled. Is null viable instead?

I checked NumericInput types, and its value prop was typed as value?: number (it doesn't accept null). So I updated NumericInput to accept number | null.

mcmcgrath13 · 2026-08-10T12:47:24Z

                            id={name}
                            name={name}
-                            value={value}
+                            value={value ?? undefined}


(nit, b): save controlled<->uncontrolled issue

mcmcgrath13 · 2026-08-10T12:48:12Z

 {
    "compilerOptions": {
        /* base vite config */
+        "allowJs": true,


(q, nb): where do we have js in src? any way we could not?

Just checked again and it was only reportWebVitals.js called in index.tsx. But the output wasn't even logged anywhere so I commented out the line in index and removed the option in tsconfig

heads up that I just deleted reportWebVitals.js, so once you merge in main, that should also go away

mcmcgrath13 · 2026-08-10T12:49:15Z

            "*": ["./src/*"]
-        }
+        },
+        "types": ["vite/client", "node"]


(q, b): this code won't run in a node environment - why do we need to include node in the types?

mcmcgrath13 · 2026-08-10T12:49:36Z


        /* nbs additions */
        "plugins": [{ "name": "typescript-plugin-css-modules" }],
+        "ignoreDeprecations": "6.0",


(q, nb): what sort of things is this ignoring? should we create a follow on ticket to deal with them?

I'm not sure. I removed it.

mcmcgrath13 · 2026-08-10T12:52:28Z

        "target": "ES2020",
        "useDefineForClassFields": true,
-        "lib": ["ES2020", "DOM", "DOM.Iterable"],
+        "lib": ["ES2023", "DOM", "DOM.Iterable"],


(q, b): reading https://stackoverflow.com/questions/42093758/need-clarification-of-the-target-and-lib-compiler-options it seems like the lib appeases typescript, but doesn't guarantee polyfills are available for things that are in 2023, but not in our target of 2020 - do we know if vite or something else includes those polyfills for us? or is it safer to leave this at 2020?

This was an interesting topic to investigate. In the end I included @vitejs/plugin-legacy to add in the polyfills then terser to minimize

do we need the terser? looks like vite auto-minifies using oxc

https://vite.dev/config/build-options#build-minify

ah, I see it's not in the version of vite we have right now

https://vite.dev/config/build-options#build-minify

do we need to set it as a build option for minification? maybe with a comment as to why and that we can probably remove once we update vite?

hmm I don't think we need a separate minifier. Looks like to me that the default for vite version 8+ is oxc and we're on "vite": "^8.0.16". So I'll remove terser.

# Conflicts: # apps/modernization-ui/src/index.tsx

mcmcgrath13 · 2026-08-11T15:11:37Z


-const asTableRow = (condition: Condition): TableBody => ({
-    id: condition.id,
+type ConditionTableBody = TableBody & { id: number };


I lead you astray here, condition's id is absolutely a string (and not always a number at all (e.g. DE-41665 is in our db) - we should have this be string and make sure the usage matches all that - apologies 🙏

mcmcgrath13 · 2026-08-11T18:53:12Z

-    const handleAddConditions = (conditions: number[]) => {
+    const handleAddConditions = (conditions: string[]) => {
        const newConditions = conditions
            .map((id) => String(id))


(nit): shouldn't need this map anymore

Suggested change

.map((id) => String(id))

mcmcgrath13

🧹

brick-green added 24 commits August 4, 2026 09:32

fix handleSelect in ConditionTable.tsx

d9e1ceb

set type for selectField obj

b3c57c2

update toBeCalledWith to toHaveBeenCalledWith

931c2a9

add type for callback on handleExternalOnBlur

513ad59

add parens to fn

c72a7e4

set types on params for handleFieldOnChange

3e239ae

dateRangeFilter updates

994faa0

null coalesce the value on NumericInput

5b901bd

null coalesce on Modal id

e501745

mock filter missing required defaultIncludeNulls

21b6d69

null coalesce NumericInput value

1dfb52f

ValueEditorSwitch pass any type for default

25ed0e7

replace toBeCalled with toHaveBeenCalled

ca3a281

validateNumericRange expects string array

9963990

cast type as QbQuery

7a422ae

add parens to fn

1ff73ce

update toBeCalledTimes to toHaveBeenCalledTime and lastCalledWith to …

4ed430f

…toHaveBeenLastCalledWith

mock the modalRef

664d2ce

remove unused React import

2ff5171

typecheck script

da4c730

address remaining failures

9cdbdd6

add ci type check

36c424d

Merge branch 'main' into bg/app-917/automated-typescript-check

585e599

mcmcgrath13 reviewed Aug 4, 2026

View reviewed changes

Comment thread apps/modernization-ui/tsconfig.app.json Outdated

Merge branch 'main' into bg/app-917/automated-typescript-check

518ff11

mcmcgrath13 reviewed Aug 4, 2026

View reviewed changes

Comment thread apps/modernization-ui/tsconfig.app.json Outdated

brick-green added 3 commits August 5, 2026 09:27

lint fix

bf0d2f1

valueInput accepts extended FullField

b9717d8

fix remaining lint and typecheck

23a5cfe

mcmcgrath13 reviewed Aug 10, 2026

View reviewed changes

brick-green added 12 commits August 10, 2026 11:13

meaningful alts

4a75634

test id on ManageSection.spec.tsx

079c902

remove as DbQuery

315b093

object to Record<string, unknown>

937de00

update NumericInput value type to number | null

7022e8b

remove use of deprecations and js in config

d513128

add @vitejs/plugin-legacy and terser to add polyfills

d0fa40f

Merge branch 'main' into bg/app-917/automated-typescript-check

d312ba2

lint fix

5d5744f

prettier format

0477710

remove use of screen in checkbox test

96599c2

Merge branch 'main' into bg/app-917/automated-typescript-check

02b8143

# Conflicts: # apps/modernization-ui/src/index.tsx

brick-green requested a review from mcmcgrath13 August 11, 2026 15:01

mcmcgrath13 reviewed Aug 11, 2026

View reviewed changes

brick-green added 3 commits August 11, 2026 14:17

remove terser

ba4ea9d

another use of condition table to string

2983ca1

prettier format

b26825e

brick-green requested a review from mcmcgrath13 August 11, 2026 18:50

mcmcgrath13 reviewed Aug 11, 2026

View reviewed changes

mcmcgrath13 approved these changes Aug 11, 2026

View reviewed changes

brick-green and others added 2 commits August 11, 2026 15:03

remove map to string

459df6b

Merge branch 'main' into bg/app-917/automated-typescript-check

35e1e84

brick-green merged commit 9488177 into main Aug 11, 2026
2 checks passed

brick-green deleted the bg/app-917/automated-typescript-check branch August 11, 2026 20:13

Uh oh!

Conversation

brick-green commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tickets

Checklist before requesting a review

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcmcgrath13 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

brick-green commented Aug 4, 2026 •

edited

Loading