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 FOUC of Prism Theme #7867

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

narinluangrath
Copy link

@narinluangrath narinluangrath commented Aug 1, 2022

Still a draft PR while I wait for this PR to get merged and I validate that it actually fixes the FOUC.

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

Once this PR for prism-react-renderer is merged, we can solve the FOUC issue described in @lex111's PR here.

Test Plan

Test links

Deploy preview: // TODO

Related issues/PRs

How to reproduce FOUC

  1. Build and serve docusaurus website
cd docusaurus
yarn install
yarn website:build:en
yarn serve:website
  1. Navigate to http://localhost:3000/docs/sidebar/autogenerated

  2. Toggle to dark mode

  3. Add breakpoint to top level <html>
    add-breakpoint

  4. Refresh the page (with the chrome inspector open) and observe FOUC
    fouc

How to reproduce FOCU fix

  1. Build this PR for prism-react-renderer and link it
cd prism-react-renderer
git fetch && git checkout improv/expose-css-vars-for-ssr
yarn install
yarn build
yarn link
  1. Go back to docusaurus, checkout my PR and link to local copy of prism-react-renderer
cd docusaurus
# Checkout my fork
git remote set-url origin git@github.com:narinluangrath/docusaurus.git
git checkout narinluangrath/prism-styles-fouc
yarn link prism-react-renderer
yarn install
  1. Rebuild website and serve
yarn website:build:en
yarn serve:website
  1. Repeat steps 2-4 in the previous section

  2. Refresh the page (with the chrome inspector open) and observe that styling is correct
    no-fouc

  3. Cleanup (unlink, reset origin)

git remote set-url origin git@github.com:facebook/docusaurus.git
yarn unlink prism-react-renderer

@narinluangrath
Copy link
Author

narinluangrath commented Aug 1, 2022

Build is failing because the code in this PR uses functionality from prism-react-renderer that doesn't exist yet (namely, the generateScriptForSSR named export). I should be able to get it running locally by yarn linking to the code in this PR, but I get the following error message.

image

Would appreciate some help here. Solved it by nuking node_modules.

@narinluangrath
Copy link
Author

@lex111 would appreciate your feedback/insight

@slorber
Copy link
Collaborator

slorber commented Aug 10, 2022

Great, looking forward to fix this prism FOUC issue ;)

To make the review easier, what about publishing your prism changes on npm so that we can just use your temporary published package on this PR, this would help the review.

Otherwise, I like the idea of using CSS variables. Wanted to do something similar myself, but in a more consistent way (ie get 100% rid of the inline styles, and always use CSS variables for all renders)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants