bcgov / design-system

The B.C. Design System helps public sector design and development teams build consistent, accessible products
https://gov.bc.ca/designsystem
Apache License 2.0
57 stars 38 forks source link

Unit tests for Header, Footer, Form, InlineAlert, Switch #476

Closed ty2k closed 2 months ago

ty2k commented 2 months ago

This adds unit tests for:

I found that some existing default React import statements (import React from "react") caused tests to not run. In cases where we were using methods on the React object, I have explicitly kept those imports. In cases where we were only using React for type information, I have removed the default import statement. This shouldn't make any difference - it's just to keep the TypeScript compiler happy. @mkernohanbc you had mentioned this in #450 and at that time, I couldn't remove the import statement without the compiler complaining on those React.ReactElement type fields. We haven't changed our tsconfig.json file, so I'm thinking something may have changed in one of the underlying devDependencies. 🤷‍♂️

Speaking of the TypeScript compiler, I find I can't add a test file for the Select component because of implicit types that the Jest test runner feels are missing:

Jest output for Select test

I'm not sure what the best way to fix that is yet, so there's no test file added for Select in this PR.

All tests should be passing in this PR.

mkernohanbc commented 2 months ago

Speaking of the TypeScript compiler, I find I can't add a test file for the Select component because of implicit types that the Jest test runner feels are missing:

Jest output for Select test

I'm not sure what the best way to fix that is yet, so there's no test file added for Select in this PR.

All tests should be passing in this PR.

This is weird, because all of those props have type declarations. If we explicitly re-declare/type those props within SelectProps, would it work (and is there a disadvantage to doing that beyond "this should be redundant"?)

ty2k commented 2 months ago

Speaking of the TypeScript compiler, I find I can't add a test file for the Select component because of implicit types that the Jest test runner feels are missing:

Jest output for Select test

I'm not sure what the best way to fix that is yet, so there's no test file added for Select in this PR. All tests should be passing in this PR.

This is weird, because all of those props have type declarations. If we explicitly re-declare/type those props within SelectProps, would it work (and is there a disadvantage to doing that beyond "this should be redundant"?)

Yeah, I can see the type information in VS Code when I hover those variables. It's the subtle differences in environment between VS Code's idea of the types at code time vs the TypeScript compiler's idea of the types at compile time vs Jest's idea of the types at test time. I don't understand the difference yet. Probably the solution will look like importing the types from the underlying RAC libraries (maybe react-aria-components, maybe one of the underlying state libraries like @react-stately) and then those render prop lines will look something like...

{({ isOpen, isRequired, isInvalid }: SelectStateProps) => (

... or whatever. I can't get it working yet! :)