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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Remix packages and use new Layout component #209

Open
brookslybrand opened this issue Feb 26, 2024 · 3 comments
Open

Upgrade Remix packages and use new Layout component #209

brookslybrand opened this issue Feb 26, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@brookslybrand
Copy link
Contributor

Vite is stable 馃帀

Let's update our packages to the stable version.

Also, let's add the new Layout export instead of the Document component we leveraged instead.

If you want to take this on, feel free to split these 2 PRs if that's easier.

@brookslybrand brookslybrand added enhancement New feature or request good first issue Good for newcomers labels Feb 26, 2024
@Kazuhiro-Mimaki
Copy link
Contributor

I have a question about replacing new Layout export instead of the Document component.

Is it possible to pass some props like forceDark, noIndex from App / ErrorBoundary / HydrateFallback to Layout?
It seems that Layout receive only children as the props, and I want to know how to pass other props.

@brookslybrand
Copy link
Contributor Author

@Kazuhiro-Mimaki yeah good question. It is not possible to pass in those props, so the logic would have to be reworked a bit.

Thanks for #213, that probably makes more sense as a single PR. I mentioned this to @brophdawg11 as he may want to dogfood the Layout piece, but if you have any ideas for how to rearchitect it feel free to take a stab

@brophdawg11
Copy link
Contributor

IMO the new Layout component is syntactic sugar for common use cases where the "shell" is identical between Component, ErrorBoundary, and/or HydrateFallback. That doesn't mean that every Remix app should definitely use a Layout component.

I'm not familiar with everything that the Document component is doing (yet! I will dig in a bit when I have a sec) but if App/ErrorBoundary have to make enough decisions on their own that drive the Document via props - then composing via Layout may not be the right choice and maybe we just stick with Document.

Another option is whether or not useRouteError can be the thing we fork on to make those decisions in a Layout component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants