-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[v2] Guidelines around layout changes #6127
Comments
I'm having this exact problem. I would be interested in how to solve it. Edit: So this is only a problem in |
I'm seeing the same issue with the built version: https://5b310bfdc6aed603a12b4cc7--jonbellah.netlify.com/contact/ |
Hey! I also got the same problem. I got into gatsby when it was reaaaally fresh and love it. Now, I am in the process of migrating our website from 0.x and thought "yay I'll just skip v1 and go straight to v2!" However, I do need components that don't re-render on page transition for my new Ideas. I don't see yet how this is supposed to work. To demonstrate, here is a very barebones start of the new site: live site: http://gatsby-transition-issue.volligohne.de/ The heading always flashes red when re-rendered with a css transition. The are console.log()s in the layout component when it gets constructed, mounts and unmounts you can check out in the console. Surprisingly, it does not re-render when you use the next and previos buttons. I guess because the markdown based pages share a template. For me there is no difference between development mode and the build version. Even if it does not get fixed very soon, I would be very happy if anyone got an idea how to hack around it for now so I dont have to start this new project on v1. I would love to have some kind of root or master template where all the Thanks! |
I think the way to avoid this is to use The issue here, is that the new hierarchy is |
Yeah I think using |
I still think adding a layout plugin that uses that api is the best migration path. It wouldn't handle nested layouts but should be a nice inbetween solution |
I'd be interested in taking a stab at a layout plugin. I don't have any experience with |
Okay, so I did a bit of reading through the source and thinking on an approach. I'm thinking Optionally, a different path could be specified:
Then using the Limitations that I can see with this approach would be nested layouts, as mentioned, as well as lacking support for multiple layouts. Thoughts on this approach? |
Hey everyone, thanks for the hints and ideas and discussion! I don't quite understand yet how I modified the given example in demo: http://gatsby-transition-issue-2.volligohne.de/ Maybe I am mistaken but is it maybe also not cool for performance if always everything is re-rendered? Isn't part of the idea of react to only re-render what's needed? So yes @jonbellah it seems to me such a layout plugin would be really good to have. Apart from that, if this is possible with the current v2 I would be very thankful about hints on how to do it :-) |
@timurc I may be missing it, but I don't think you're actually hooking into the I've been working off the example there, along with the tests I found in I'm still working on a proof of concept... running into some rough edges here and there, but working through them. |
Okay, I'm a little stuck. I have the But in v2, I'm getting an error -- |
One layout component could be sufficient since it could switch between layouts based on routes. Since it's a normal component, it can do nesting as needed as well. |
@m-allanson were you planning on writing |
Or @jonbellah are you interested in doing a PR? |
I verified as well that const React = require("react");
const Layout = require("./src/components/layout").default;
exports.replaceComponentRenderer = ({ props, component }) => {
console.log({ props, component });
return <Layout>{React.createElement(component, props)}</Layout>;
}; |
@KyleAMathews I'm interested in putting together a PR. I'll work one up this weekend. The code you posted is super helpful. Thanks! |
@KyleAMathews I'm a bit stumped. The Tried each of the following steps to no avail.
Always the same error - |
Hmm that sucks... I've never seen that error so not sure. Check the generated api-runner-browser.js file in .cache? |
I was looking at this earlier in the week and saw the same error (or
something very similar). I'll dig out some more details on Monday.
…On 1 July 2018 at 21:33, Kyle Mathews ***@***.***> wrote:
Hmm that sucks... I've never seen that error so not sure. Check the
generated api-runner-browser.js file in .cache?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6127 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAXTaeBoxRrD9QfOyn4aSIQob_x-WgNEks5uCTIwgaJpZM4U0-FS>
.
|
I do have the same error as @m-allanson and @jonbellah. For me it only happens when I try to import a component to Strangely, for me the example of @KyleAMathews does not work because const React = require('react');
// fails when imported: "Uncaught TypeError: (0 , _apiRunnerBrowser.apiRunner) is not a function"
// const Layout = require('./src/components/layout').default;
exports.replaceComponentRenderer = ({ props }) => {
return <StateTest>{React.createElement(props.pageResources.component, props)}</StateTest>;
};
class StateTest extends React.Component {
constructor(props) {
super(props);
console.log('constructing...'); // Does not fire on page transition!
}
render() {
const { children } = this.props;
return <div>{children}</div>;
}
} So in the example above the component |
@timurc What a find... nice work! I'll hold off on moving anymore on |
This error seems to be triggered if the component you're importing also imports Gatsby's Here's a demo repo: https://github.com/m-allanson/gatsby-v2-layout-test This |
So this is because from importing from |
I've opened a PR which fixes the However we might need to do some more thinking about how this should work. Using Maybe this calls for a couple of new APIs? something like |
@HriBB I've used |
Trying to use the latest Gatsby beta. I can't figure out a way to get a template component working with the available APIs. Closest I've got: // gatsby-ssr.js
import React from 'react'
import Layout from './src/components/Layout'
import { Location } from '@reach/router'
import { renderToString } from "react-dom/server"
export const replaceRenderer = ({
bodyComponent,
replaceBodyHTMLString,
}) => {
const newBody = (
<Location>
{props => (
<Layout {...props}>
{bodyComponent}
</Layout>
)}
</Location>
)
const newBodyString = renderToString(newBody)
replaceBodyHTMLString(newBodyString)
} // gatsby-browser.js
import React from 'react'
import Layout from './src/components/Layout'
import { Location } from '@reach/router'
export const wrapRootComponent = ({ Root }) => {
const WrappedRootComponent = props => (
<Location>
{routeProps => (
<Layout {...routeProps}>
<Root {...props} />
</Layout>
)}
</Location>
)
return WrappedRootComponent
} The problem with this is that it breaks the emotion plugin. If |
@DylanVann I'm working on solution for this. It will involve some new APIs that will allow wrapping page component in both ssr and browser and without limitation to just single use (like with your example + using emotion plugin) |
Solution here will be few parts:
|
@DylanVann try this interim, let me know if you have more questions // gatsby-browser.js
const LayoutRenderer = ({ PageComponent, pageProps }) =>
<YourLayout {...pageProps}><PageComponent {...pageProps} /></YourLayout>
export const replaceComponentRenderer = ({ props: { pageResources, ...pageProps } }) =>
<LayoutRenderer PageComponent={pageResources.component} pageProps={pageProps} /> // gatsby-ssr.js
export const replaceRenderer = ({ bodyComponent }) => {
const page = bodyComponent.children.props.children
bodyComponent.children.props.children = <YourLayout {...page.props}>{page}</YourLayout>
return bodyComponent
} |
@alpgumus That code almost works. I'm hitting: So it looks like I'd have to implement emotion SSR myself using this API. @pieh That sounds awesome, looking forward to trying it out. I think for now I'll just not upgrade to the latest beta. I'll revisit in a few weeks. |
@pieh I'm glad to see that someone's working on allowing for multiple iterations of It's not necessarily true that someone who needs one for css-in-jss stuff like |
Actually it will probably be separate API and not I did some tests with jss plugin with my new APIs and it worked nicely, but will probably need to change those because they are hard to explain (you know your API is not well designed if you can't figure out how to document it so people understand it :) ) |
This most likely be copy of Additional change would be ability to chain results from one plugin as input for next plugins, so this isn't limited to single plugin - so you could wrap root component with provider for jss and then wrap that in provider for redux and wrap that in provider for react context etc |
So first PR is opened - #7204, after that this will follow - pieh@7442f70 ( and a lot of doc changes |
Migration guide is updated to mention Next item on to-do list is to put this information somewhere else in documentation, so people can discover if they are not migrating from v1 and need this behaviour. |
Is there a solution for nested layouts? |
Same question as @dallanlee. My situation: I have a One of my page types (created via Possible solutionI could move To avoid the extra JS payload, I could make Even if I do, this feels like a suboptimal solution. Among other reasons, pulling the The "proper" solution, it seems, would be to simply rerender (rather than re-mount) the page component when the path changes, as long as the React component is the same between paths. Thoughts? |
@jessepinho I had a similar issue whereas my front page has a slightly different layout than the rest. I was able to use the |
@cyberwombat the issue is that the code for both layouts is always loaded, in that case — which means a lot of extra bytes of JS sent to the browser. |
Summary
It's possible that I may be misinterpreting the documentation, but it appears that the suggested method of wrapping pages with a layout component causes those pages to re-render completely upon navigation.
Relevant information
The docs for layout in v2 say:
See: https://next.gatsbyjs.org/docs/migrating-from-v1-to-v2/#update-layout-component
I've followed the guide, wrapping each of the pages in my
/pages/
directory with my new Layout component. I have a little slide down animation on my header on initial page load... following the new<Layout />
component guidelines causes that to animate every time the page changes.I understand I can use the
html.js
file or render routes directly from react-router inside my<Layout />
, but I figured I'd surface the issue I found with the docs (or my misunderstanding of the docs, so that I can help bring a bit more clarity for others following the guide soon).The code for this can be found here: https://github.com/jonbellah/jonbellah.com/tree/v2
Relevant pieces of code:
Please let me know if I can provide any other clarity. I'm interested in contributing back to the docs, if this proves to be something worth addressing there.
The text was updated successfully, but these errors were encountered: