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.

Sign up for GitHub

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

Jump to bottom

Support an optional Layout export from the root route #8709

Merged
merged 6 commits into from
Feb 8, 2024
Merged

Support an optional Layout export from the root route #8709

merged 6 commits into from
Feb 8, 2024

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Feb 7, 2024 •
edited
Loading

RFC #8702
Closes #8703

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: c5826ce

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/dev Minor
@remix-run/react Minor
@remix-run/server-runtime Minor
@remix-run/testing Minor
@remix-run/cloudflare Minor
@remix-run/deno Minor
@remix-run/node Minor
@remix-run/cloudflare-pages Minor
@remix-run/cloudflare-workers Minor
@remix-run/architect Minor
@remix-run/express Minor
@remix-run/serve Minor
create-remix Minor
remix Minor
@remix-run/css-bundle Minor
@remix-run/eslint-config Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

brophdawg11
brophdawg11 commented Feb 8, 2024
View reviewed changes
docs/file-conventions/root.md
@@ -64,16 +79,98 @@ export default function App() {
}
```

You can pass extra props to `<Scripts />` like `<Scripts crossOrigin />` for hosting your static assets on a different server than your app.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this and the nonce section below over to the <Scripts> doc

@brophdawg11 brophdawg11 linked an issue Feb 8, 2024 that may be closed by this pull request
🗺️ Root Layout #8703
Closed
@brophdawg11 brophdawg11 merged commit c9615ca into dev Feb 8, 2024
9 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/layout-export branch February 8, 2024 16:47
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Feb 8, 2024
@github-actions github-actions bot mentioned this pull request Feb 16, 2024
Merged
@github-actions GitHub Actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.7.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions github-actions bot mentioned this pull request Feb 20, 2024
Merged
@github-actions GitHub Actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.7.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions github-actions bot removed the awaiting release This issue has been fixed and will be released soon label Feb 20, 2024
@aaronadamsCA
Copy link
Contributor

Does this require the Vite dev server?

In my usage so far it works fine with remix vite:dev, but with remix dev the hydrated document is not wrapped with the Layout component.

I'm not sure if you want a bug report, or if you would rather just document this limitation. It probably depends on how aggressively you want to drop non-Vite support.

@aaronadamsCA aaronadamsCA mentioned this pull request Feb 23, 2024
Closed
@ngbrown
Copy link
Contributor

ngbrown commented Mar 5, 2024 •
edited
Loading

I currently use the <html className /> for setting the user preferred dark class. It should also match the rendered state, so it is synchronized between default browser preference and server with a cookie. Even setting this in <body className /> still needs additional context to be rendered from the Layout component.

I can't set it at any lower of a level than body and still have headless-portal-root elements pick up the dark class.

I also need to set the nonce on the <Scripts /> and <LiveReload /> components. What if a site needed to have lang besides <html lang="en" />?

Is there a possibility to pass more than just children to the Layout component?

@brophdawg11
Copy link
Contributor Author

Is there a possibility to pass more than just children to the Layout component?

No, Layout is the top-most component so there's nowhere to determine prop values and pass them through. Layout is really just syntactic sugar for the simple case where the app shell is shared between ahppy and error-path UIs. If you need more complex differences than you may be better off not using Layout and using your own component that accepts props. However, chances are you're going to be determining those prop values from useLoaderData and useRouteError and could just as easily grab that info from useRouteLoaderData("root") and useRouteError in Layout

@ngbrown
Copy link
Contributor

ngbrown commented Mar 6, 2024

grab that info from useRouteLoaderData("root") and useRouteError in Layout

Thanks, useRouteLoaderData("root") was the missing link for me. It gave me the added benefit of having the root ErrorBoundary have more of the configuration from the root loader (if the root loader isn't what crashed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed package:dev renderer:react
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🗺️ Root Layout
3 participants
@brophdawg11 @aaronadamsCA @ngbrown