Skip to content

Conversation

@metalwarrior665
Copy link
Member

@metalwarrior665 metalwarrior665 commented May 9, 2025

Prerequisite apify/apify-docs#1569 was merged

Questions:

  1. The string -> Date conversion happens automatically for all types, right? So no need to do anything?
  2. Do we use explicit T | null anywhere if the API returns the type or null? Seems we like to do T? rather

@B4nan
Copy link
Member

B4nan commented May 13, 2025

The string -> Date conversion happens automatically for all types, right? So no need to do anything?

I believe it works automatically on keys suffixed with At, so createdAt should work fine. Best way to be sure is to write a test case (or at least try it out locally with the changes).

Do we use explicit T | null anywhere if the API returns the type or null? Seems we like to do T? rather

I personally prefer using optional fields (so undefined) for better ergonomics, but if the API returns null somewhere, we need to have it in the type, and if a user can pass null somewhere explicitly, we need to allow it in that type too.

Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

looks good, but please ensure the nulls are correct (so if the API can return null somewhere, we have it on type level, if not and the API would instead omit the property, it shouldn't be there)

@metalwarrior665
Copy link
Member Author

I tested the Date conversion locally and double-checked the explicit nulls

@metalwarrior665 metalwarrior665 merged commit e138093 into master May 13, 2025
7 checks passed
@metalwarrior665 metalwarrior665 deleted the fix/add-new-user-private-fields branch May 13, 2025 10:47
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.

3 participants