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(rsc): Load all css links to support css with rsc #10544

Merged
merged 8 commits into from
May 11, 2024

Conversation

Josh-Walker-GM
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM commented May 7, 2024

Problem
Currently we don't have good css support with RSC. Therefore the pages are unstyled and look ugly.

Changes

  1. We parse the build manifests and collect all css assets that could be loaded. We then pass these through to the Document component which ultimately causes them to be inserted into the HTML stream as link tags.
  2. Removes the vite plugin which used to add preinit calls. It's no longer used and can be revived from the git history if needed.

Notes

  1. This link was helpful in confirming that this isn't a crazy road to be going down: https://vitejs.dev/guide/backend-integration.html#backend-integration
  2. We can of course optimise in the future. We can go back to trying to get preinit working or we can stop loading some of these css assets by determining if they need to be loaded or not based on the url we're responding to.

@Josh-Walker-GM Josh-Walker-GM added release:fix This PR is a fix changesets-ok Override the changesets check labels May 7, 2024
@Josh-Walker-GM Josh-Walker-GM added this to the RSC milestone May 7, 2024
@Josh-Walker-GM Josh-Walker-GM self-assigned this May 7, 2024
@Josh-Walker-GM Josh-Walker-GM marked this pull request as ready for review May 7, 2024 02:35
@Josh-Walker-GM Josh-Walker-GM requested a review from Tobbe May 7, 2024 02:35
@Josh-Walker-GM Josh-Walker-GM added the fixture-ok Override the test project fixture check label May 7, 2024
Comment on lines +159 to +162
createElement(ServerEntry, {
location: { pathname: rscId },
css: cssLinks,
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

Really nice implementation 🥇

Copy link
Collaborator Author

@Josh-Walker-GM Josh-Walker-GM May 11, 2024

Choose a reason for hiding this comment

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

Thanks! The way that Tobbe and Danny laid the groundwork here made it somewhat easy to pass the collected css links through and have them loaded.

Copy link
Contributor

@peterp peterp left a comment

Choose a reason for hiding this comment

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

This seems like a solid implementation to me! But will defer the final decision/ review to @Tobbe.

@Josh-Walker-GM Maybe it would be wise to keep a note of where/how we could potentially improve performance (Like you've done in the PR), but in the code?

@Josh-Walker-GM Josh-Walker-GM enabled auto-merge (squash) May 11, 2024 13:57
@Josh-Walker-GM Josh-Walker-GM merged commit 464ee74 into main May 11, 2024
46 checks passed
@Josh-Walker-GM Josh-Walker-GM deleted the jgmw/rsc-css-inject-to-document branch May 11, 2024 14:07
dac09 added a commit that referenced this pull request May 15, 2024
…uth-store

* 'main' of github.com:redwoodjs/redwood:
  chore(router): Add more code comments to analyzeRoutes (#10569)
  RSC: No need to use memo or useMemo in the server router (#10568)
  RSC: getViteConfig in rscWorker (#10567)
  Split RSC and RSA handling in rscWorker (#10565)
  RSC: Extract the fetchRSC function (#10564)
  RSC: Fix TODO regarding SSR in client.ts (#10562)
  Docs: QoL Updates to Contributing.md (#10561)
  Added warning note to not add WebAuthn (#10231)
  fix(rsc): Load all css links to support css with rsc (#10544)
  fix(cli): Add deprecation notice for edgio deployment (#10551)
  Define `process.env.NODE_ENV` in build process. (#10553)
  chore(docs): Update dbAuth and Supabase middleware READMEs (#10552)
  chore(deps): Upgrade React 19 to beta 20240508 (#10560)
  Revert "chore(deps): React beta 20240508 (#10558)" (#10559)
  chore(deps): React beta 20240508 (#10558)
  fix(functions): Fix context variable warning/error (#10556)
  fix(functions): Mock context in function test template (#10555)
  chore(middleware): Format code and comments and fix comment grammar (#10554)
@Josh-Walker-GM Josh-Walker-GM modified the milestones: RSC, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changesets-ok Override the changesets check fixture-ok Override the test project fixture check release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants