-
Notifications
You must be signed in to change notification settings - Fork 94
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
Remove date-fns dependency and reduce bundle size #163
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@jahirfiquitiva is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
@@ -1,9 +1,9 @@ | |||
'use client' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking this could probably no longer be a client
component.
We're using a unique format for the date, and the tweet date will not change. 🤔
Open to discuss about it.
I created a custom server component for my website, implementing these changes so date-fns
was not bundled, and it works just fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was initially use client
because the previous version of the X embedded tweet had relative times (like 5 days ago) and calculating the relative time in the server would end up in a cached and incorrect result.
Due to how it is today, it's most likely safe to make a server component and it's actually better to avoid running the logic behind the formatter in the browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw that there's a comment in the component that talks exactly about this so yeah, it's better to make it a server component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. I Added some minor changes and it looks good to go!
My PR got merged! 🎉 (vercel/react-tweet#163)
This PR removes
date-fns
(~17.2 kB) and instead usesIntl.DateTimeFormat
formatToParts
, while keeping the same functionality and date format.This could be a follow-up PR to #162 . I decided to create a new PR instead of updating the other one just so you could consider both options. I personally like this one better, as it's not using any dependency.
Also, special thanks to @arturocr for his comment and explanation which helped writing this PR.
Before
vite-app
create-react-app
custom-tweet-dub
next-app
After
vite-app (~12.02% smaller)
create-react-app (~17.89% smaller)
custom-tweet-dub (~6.39% smaller)
next-app (~6.82% smaller)