-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
refactor(theme): dates should be formatted on the client-side instead of in nodejs code #9868
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: 0 B Total Size: 992 kB ℹ️ View Unchanged
|
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.
Looks like it's working
However the React code is too duplicated, we need a better abstraction.
Yes existing tests are failing, all the tests/snapshots need to be cleaned up from this removed nodejs attribute 😄
Note: we also have a formatted date in the docs plugin.
I think this refactor is a good opportunity to also migrate this code to the client-side:
formattedLastUpdatedAt: lastUpdate.lastUpdatedAt
? formatDate(
i18n.currentLocale,
new Date(lastUpdate.lastUpdatedAt * 1000),
i18n.localeConfigs[i18n.currentLocale]!.calendar,
)
: undefined,
packages/docusaurus-theme-classic/src/theme/BlogArchivePage/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/BlogArchivePage/index.tsx
Outdated
Show resolved
Hide resolved
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
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.
Looks almost good 👍
packages/docusaurus-plugin-content-blog/src/__tests__/index.test.ts
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/BlogArchivePage/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/BlogPostItem/Header/Info/index.tsx
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/LastUpdated/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-plugin-content-blog/src/__tests__/index.test.ts
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/LastUpdated/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/LastUpdated/index.tsx
Outdated
Show resolved
Hide resolved
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.
LGTM thanks 👍
… of in nodejs code (#9868) Co-authored-by: OzakIOne <[email protected]> Co-authored-by: sebastien <[email protected]>
Pre-flight checklist
Motivation
As discussed in #9830 this would allow users to have their own format date by swizzling
However I'm wondering if it is the correct way to do it and I also don't know what to do with the failing tests because the formatted date is no more in node
Test Plan
Unit test
Test links
Deploy preview: https://deploy-preview-9868--docusaurus-2.netlify.app/blog/archive
Related issues/PRs
fix #9830