Skip to content

Comments

refactor(theme): dates should be formatted on the client-side instead of in nodejs code#9868

Merged
slorber merged 8 commits intomainfrom
ozaki/themecommon-formatdate
Feb 23, 2024
Merged

refactor(theme): dates should be formatted on the client-side instead of in nodejs code#9868
slorber merged 8 commits intomainfrom
ozaki/themecommon-formatdate

Conversation

@OzakIOne
Copy link
Contributor

@OzakIOne OzakIOne commented Feb 19, 2024 •
edited
Loading

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

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

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Feb 19, 2024
@netlify
Copy link

netlify bot commented Feb 19, 2024 •
edited
Loading

✅ [V2]

Name Link
🔨 Latest commit 220a50f
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/65d8c0ac67bdac0008e24046
😎 Deploy Preview https://deploy-preview-9868--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions
Copy link

github-actions bot commented Feb 19, 2024 •
edited
Loading

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 76 🟢 98 🟢 96 🟢 100 🟠 88 Report
/docs/installation 🟠 88 🟢 96 🟢 100 🟢 100 🟠 88 Report
/docs/category/getting-started 🟠 75 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog 🟠 70 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 63 🟢 96 🟢 100 🟢 100 🟠 88 Report
/blog/tags/release 🟠 71 🟢 100 🟢 100 🟠 80 🟠 88 Report
/blog/tags 🟠 76 🟢 100 🟢 100 🟢 90 🟠 88 Report

@github-actions
Copy link

github-actions bot commented Feb 19, 2024 •
edited
Loading

Size Change: 0 B

Total Size: 992 kB

ℹ️ View Unchanged
Filename Size
website/.docusaurus/globalData.json 75.4 kB
website/build/assets/css/styles.********.css 114 kB
website/build/assets/js/main.********.js 765 kB
website/build/index.html 37.9 kB

compressed-size-action

slorber
slorber requested changes Feb 20, 2024
View reviewed changes
Copy link
Collaborator

@slorber slorber left a comment •
edited
Loading

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,

@slorber slorber changed the title feat(theme-common): format date refactor(theme): dates should be formatted on the client-side instead of in nodejs code Feb 20, 2024
@slorber slorber added pr: polish This PR adds a very minor behavior improvement that users will enjoy. to backport This PR is planned to be backported to a stable version of Docusaurus Argos Add this label to run UI visual regression tests. See argos.yml GH action. labels Feb 22, 2024
@argos-ci
Copy link

argos-ci bot commented Feb 22, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 🧿 Changes detected (Review) 1 change Feb 23, 2024, 4:07 PM

slorber
slorber requested changes Feb 22, 2024
View reviewed changes
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Looks almost good 👍

@slorber slorber mentioned this pull request Feb 22, 2024
Merged
3 tasks
@OzakIOne OzakIOne marked this pull request as ready for review February 23, 2024 15:36
@OzakIOne OzakIOne requested a review from Josh-Cena as a code owner February 23, 2024 15:36
slorber
slorber requested changes Feb 23, 2024
View reviewed changes
@OzakIOne OzakIOne requested a review from slorber February 23, 2024 15:58
slorber
slorber approved these changes Feb 23, 2024
View reviewed changes
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

LGTM thanks 👍

@slorber slorber merged commit 0279c32 into main Feb 23, 2024
@slorber slorber deleted the ozaki/themecommon-formatdate branch February 23, 2024 17:30
OzakIOne added a commit that referenced this pull request Feb 28, 2024
@OzakIOne @slorber
refactor(theme): dates should be formatted on the client-side instead…
c737407 
… of in nodejs code (#9868)

Co-authored-by: OzakIOne <[email protected]>
Co-authored-by: sebastien <[email protected]>
OzakIOne added a commit that referenced this pull request Mar 4, 2024
@OzakIOne
Revert "refactor(theme): dates should be formatted on the client-side…
13c4b5e 
… instead of in nodejs code (#9868)"

This reverts commit c737407.
@guycherno guycherno mentioned this pull request May 19, 2024
Open
@qdraw qdraw mentioned this pull request Jun 22, 2024
Merged
@kaelynj kaelynj mentioned this pull request Jun 23, 2024
Closed
This was referenced Jun 27, 2024
Open
Open
This was referenced Jul 17, 2024
Merged
Merged
Merged
Merged
This was referenced Jul 21, 2024
Closed
Closed
This was referenced Jul 21, 2024
Open
Open
Open
This was referenced Aug 4, 2024
Merged
Closed
@Abuchtela Abuchtela mentioned this pull request Aug 11, 2024
Closed
This was referenced Aug 12, 2024
Closed
Closed
This was referenced Aug 15, 2024
Closed
Closed
Closed
This was referenced Aug 27, 2024
Open
Open
Open
Open
Open
@Ramyromel Ramyromel mentioned this pull request Aug 28, 2024
Merged
@Herychukwu Herychukwu mentioned this pull request Aug 31, 2024
Open
This was referenced Sep 3, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slorber slorber slorber approved these changes

@Josh-Cena Josh-Cena Awaiting requested review from Josh-Cena Josh-Cena is a code owner

Assignees

No one assigned

Labels

Argos Add this label to run UI visual regression tests. See argos.yml GH action. CLA Signed Signed Facebook CLA pr: polish This PR adds a very minor behavior improvement that users will enjoy. to backport This PR is planned to be backported to a stable version of Docusaurus

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Hide redundant year in dates shown in the Archive page

3 participants

@OzakIOne @slorber @facebook-github-bot