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 getFieldset with interfaces and nullable schemas #508

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

aaronadamsCA
Copy link
Contributor

@aaronadamsCA aaronadamsCA commented Mar 3, 2024

This should fix #499, specifically the types failing to reveal getFieldset in these two scenarios:

  • Your FieldSchema is an interface, not a type.
  • Your defaultValue is nullable.

Known existing issues this PR doesn't try to fix:

  • getFieldset is incorrectly available on other object types, e.g. Date.

I'm filing this as a draft, as I have no idea what kinds of tests to write for this.

@aaronadamsCA
Copy link
Contributor Author

aaronadamsCA commented Mar 3, 2024

Ah, okay, I see now that these are tested via a playground package type check, so now I know what to run locally to test changes.

I fixed a very weird problem caused by TypeScript quirks:

  • NonNullable is implemented as T & {}, since {} means "any non-nullish value".
    • Therefore NonNullable<unknown> resolves to {}
  • But {} extends object resolves to true, probably for legacy reasons.
    • Therefore NonNullable<unknown> extends object resolves to true (yikes!).

So I stuck with Exclude, which works just fine.

I also updated the tests to use interface instead of type. They fail without this change and pass with it.

Copy link
Owner

@edmundhung edmundhung left a comment

Choose a reason for hiding this comment

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

Thank you!

@edmundhung edmundhung merged commit f74537f into edmundhung:main Mar 7, 2024
2 checks passed
@aaronadamsCA aaronadamsCA deleted the issue-499 branch March 8, 2024 15:38
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.

Property 'getFieldset' does not exist on type 'FieldMetadata'
2 participants