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

fix: expand typescript support to null when allowNull is true #552

Merged
merged 3 commits into from
May 2, 2024

Conversation

arthurpankiewicz
Copy link
Collaborator

@arthurpankiewicz arthurpankiewicz commented May 1, 2024

Description

  • when an attribute is not required (allowNull) then the coerce function does not allow undefined, but the frontend was only supporting <type> | undefined with typescript
  • expanded typescript to be <type> | null | undefined
    • even though undefined will throw an error in coerce function, it is required so that the field can be omitted entirely, ie. createOne({ name: undefined }) will throw an error, but omitting the attribute entirely createOne({}) works. TS cannot tell the difference between these 2

Test Plan

  • manually tested

Screenshot 2024-05-01 at 3 07 28 PM
Screenshot 2024-05-01 at 3 07 33 PM

Definition of done

  • Does new code have 90% testing coverage (100% for core) of both unit and E2E tests?
  • Is code secure? If applicable, add security notes to the description.
  • Do all new TODO comments have Jira links with enough information?
  • Has the browser error-console been reviewed to not contain new errors introduced by these code changes?
  • The site looks “good” above 1000px width.
  • The site looks “good” when the window height is small. No double scrollbars.
  • Were the changes tested to ensure 508 compliance?

@arthurpankiewicz arthurpankiewicz requested a review from roy-coder May 1, 2024 22:11
@arthurpankiewicz arthurpankiewicz merged commit 7396233 into main May 2, 2024
33 checks passed
@arthurpankiewicz arthurpankiewicz deleted the fix/fe-allow-null-typescript-support branch May 2, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants