sequelize-ui
sequelize-ui copied to clipboard
Adding question mark to optional TS Declares
When a field is not necessarily selected, typescript can be supported by adding a question mark to its type.
Old
declare targetUrl: string | null
New
declare targetUrl?: string | null
Like this
Just want to make sure I understand the use case completely. The change you proposed in https://github.com/tomjschuster/sequelize-ui/pull/87/files#diff-0087e8086054c2718d634a0e5ccbbc9b200321aa02c50bab4fbb262b65e67fef makes nullable attributes optional, but here we say:
When a field is not necessarily selected
which seems like we could be referring to literally SELECT
ing only certain fields, such as:
Post.findAll({ attributes: ['id', 'title']})
Do you mean to add support to make making nullable fields also accept undefined
(or omit the property on creation), or to support the findAll({ attributes: [...]})
use case?
If it's the former, I don't think this is necessary, since Sequelize added the utility types InferCreationAttributes
, CreationOptional
and CreationAttributes
which automatically make these fields optional for create
and build
, which seems to me to be the only place where this would be useful.
If it's the latter, then it seems like we'd need to make all fields optional, because given a Post
, there's no way to know which fields were selected (unless Sequelize were able to support omitting non-selected attributes for the return type from the query methods, which it doesn't do, and I'm not sure how feasible this would be).
I think the way Sequelize works with typescript, anytime you use attributes
with a query, you'll always be sacrificing some type safety, and you'd probably be better off mapping the result to some other type, or creating another Sequelize model that maps to the same table with less fields. Otherwise all model instance methods and functions that accepts the model need to be defensive around any given field being selected (or unsafely assume they have been):
type IdAndTitle = {id: number, title}
const data: IdAndTitle[] = await Post.findAll({ attributes: ['id', 'title'] })
// or
const data: IdAndTitle[] = (await Post.findAll({ attributes: ['id', 'title'] })).map(
({ postId, title }) => ({ postId, title }),
Thanks for the suggestion. Going to close this as I have no plans to implement this at this time. Please let me know if you have any thoughts about what I mentioned in https://github.com/tomjschuster/sequelize-ui/issues/85#issuecomment-1141517691.