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

Expose PartialDateString type #30

Merged
merged 2 commits into from
Aug 3, 2023
Merged

Conversation

MattIPv4
Copy link
Member

@MattIPv4 MattIPv4 commented Aug 3, 2023

Open to better ways to structure this, but wanted to have a central place to expose some of our more generic types (so they can easily be referenced from site code etc.)

@MattIPv4 MattIPv4 force-pushed the MattIPv4/expose-partial-date-string branch from 48a2162 to 9ec2df2 Compare August 3, 2023 02:42
@MattIPv4 MattIPv4 requested a review from pjeweb August 3, 2023 02:46
Copy link
Member

@pjeweb pjeweb left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Only thing I recently learned ist that using .d.ts for “normal” types is an anti-pattern. So maybe I would probably just name it types.ts. microsoft/TypeScript#52593 (comment)
https://youtu.be/zu-EgnbmcLY

@MattIPv4
Copy link
Member Author

MattIPv4 commented Aug 3, 2023

TIL about the implication that had for a .js file existing (gotta love that TS makes so many assumptions about .js files when it works with .ts files).

@MattIPv4 MattIPv4 requested a review from pjeweb August 3, 2023 11:13
@MattIPv4 MattIPv4 merged commit bc51ef3 into main Aug 3, 2023
6 checks passed
@MattIPv4 MattIPv4 deleted the MattIPv4/expose-partial-date-string branch August 3, 2023 11:28
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