Skip to content

Resolve date discrepancy and remove tinytime dependency #1812

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

Merged
merged 3 commits into from
Apr 1, 2024

Conversation

brandonmcconnell
Copy link

👋🏼 NOTE: I am reopening this as a follow-up to #1811. I closed that one thinking that I might need to install another dependency to polyfill Intl.DateTimeFormat for server use, but that does not appear to be the case.

I'm unable to inspect the failures that caused the previous PR's build to fail, as I am not part of the tailwindlabs org on Vercel and do not have access to the tailwindlabs/tailwindcss-com resource.

If you report the failures here, I can take a stab at resolving them.


This PR will…

  1. resolve a date mismatch issue caused by tinytime not preserving timezone information

    I noticed this when sharing the job app links on Twitter. I'm not sure if this occurs in every time zone, or if I'm only seeing it because I'm currently traveling in Los Angeles.

  2. refactor and improve the formatDate function

    1. refactor and improve the formatDate function to build upon built-in APIs

      Most of what tinytime was being used for can be offloaded to Intl.DateTimeFormat, which is natively supported with great browser support.

      The only regression/difference I noticed was that in the case of the job applications usage of formatDate, you lose the ordinal day number (e.g. 4 now instead of 4th). If that's an important difference and you really want the ordinal number there, I have a helper that essentially shims 'ordinal support for the day number. It would just be a deviation from the built-in Intl.DateTimeFormatOptions type, so if you do any TS type checking, you'll want to account for that as well.

    2. removes the—now unused—the tinytime dependency

@brandonmcconnell
Copy link
Author

To clarify—as I didn't mention this before—I ran the build process locally, and everything built without any errors, and I can see the solution correctly rendering when running my fork locally:

@adamwathan
Copy link
Member

Thanks man! Here's a slice of the build errors, looks like this error is happening for most if not all blog posts:

image

@thecrypticace
Copy link
Contributor

@adamwathan The shortGeneric error is because our Vercel build is still using Node 16 which we should change to be at least Node 18.

@adamwathan
Copy link
Member

Switched to Node 20 and triggered a redeploy, cross your fingers! 😄

Comment on lines 14 to 15
hour: 'numeric',
minute: 'numeric',
Copy link
Member

Choose a reason for hiding this comment

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

I think need to drop these in order to match the behavior of the live site:

image

Copy link
Author

Choose a reason for hiding this comment

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

Ah yup! I updated those last and must've copied the wrong object. Updating that now.

Copy link
Author

Choose a reason for hiding this comment

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

Should be resolved now via 42d3fde

Copy link

vercel bot commented Apr 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tailwindcss-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2024 4:51pm

@adamwathan
Copy link
Member

Awesome thanks @brandonmcconnell!

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