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

feat: infer_null_optionality config option #15

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aaronrosenthal
Copy link

@aaronrosenthal aaronrosenthal commented Jan 6, 2024

Description 📖

Add support for generating TypeScript interfaces with null types for optional attributes.

Background 📜

null types are generally more common for APIs because JSON does not serialize undefined.

The Fix 🔨

Added support to specify optionality as one of true, :null, or :undefined_or_null for more flexibility. Also included a infer_null_optionality configuration option (disabled by default for backward compatibility) which will allow the library to determine :null or :undefined_or_null where appropriate based on the SQL schema and the use of conditional attributes.

Screenshots 📷

@aaronrosenthal aaronrosenthal changed the title feat: null_optionality config option feat: infer_null_optionality config option Jan 6, 2024
@aaronrosenthal
Copy link
Author

@ElMassimo Wanted to check in to see if this is something you would be interested in merging?

@ElMassimo
Copy link
Owner

ElMassimo commented Sep 9, 2024

Hi Aaron!

I like this enhancement as it has the potential to improve the accuracy of the generated types.,

I'd like to use it more before committing to this API.


I'm not sure about allowing a user to define optional with different values.

I'd be more comfortable if this opt-in behavior globally, using fixed rules such as:

  • If the attribute is conditional (if:)
    • If nullable in the db or optional: true, use null | undefined
    • Else, undefined
  • Else
    • If nullable in the db or optional: true, use null

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