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

fix(theme-classic): Debounce during onRouteUpdate in classic theme #10037

Closed

Conversation

seyoon20087
Copy link
Contributor

@seyoon20087 seyoon20087 commented Apr 10, 2024

Fixes #10036.

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

Debouncing instead of using window.setTimeout() fixes this issue.

Test Plan

See PoC (the loading bar does not appear in development anymore):

Screen.Recording.2024-04-10.at.8.55.14.PM.mov

Test links

Deploy preview: https://deploy-preview-10037--docusaurus-2.netlify.app/

Related issues/PRs

#10036

Using window.setTimeout may cause the bar to remain
in StrictMode (in my testing).

Signed-off-by: seyoon20087 <[email protected]>
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 10, 2024
Copy link

netlify bot commented Apr 10, 2024

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 3969ee0
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/661681dd7e29dd000810e722
😎 Deploy Preview https://deploy-preview-10037--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.

Copy link

github-actions bot commented Apr 10, 2024

⚡️ Lighthouse report for the deploy preview of this PR

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

Forgot to run prettier before pushing

Signed-off-by: seyoon20087 <[email protected]>
@slorber
Copy link
Collaborator

slorber commented Apr 11, 2024

Thanks for your proposal, but Docusaurus does not support strict mode yet, and we don't want to introduce lodash in the client-side bundle

I'll figure this out when we add strict mode support, but for now it is not useful to fix a bug that we don't have in the first place. We'll fix problems once we support strict mode, as part of the PR that brings support for it.

@slorber slorber closed this Apr 11, 2024
@slorber slorber added the closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) label Apr 11, 2024
@seyoon20087 seyoon20087 deleted the fix-race-condition-classic-theme branch June 22, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When testing on StrictMode, the bar remains on the top
3 participants