search

remix-run / remix-website

327 stars 74 forks source link

Upgrade Remix packages and use new Layout component #209

Open brookslybrand opened 6 months ago

brookslybrand commented 6 months ago

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.

Kazuhiro-Mimaki commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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.