improve support for passing props down to children from a layout...

[busticated]

from a discussion over in discord...

say you want to pass some props down to a child component from within a layout (e.g. layouts/index.js):

const Layout = props => {
    return (
        <div>
            /*
            {props.children()} // works 
            {props.children({ ...props, foo: true })} // works, but is it safe?
            {props.children({ foo: true })} // fails with: "TypeError: Cannot read property 'listen' of undefined"
            */
        </div>
    )
}

digging into the code, i think the issue is here - specifically:

const props = layoutProps ? layoutProps : routeProps
attachToHistory(props.history)

so when we pass down props { foo: true } from the layout, props.history is now undefined.

short-term, i'm wondering if there's any downside to always forwarding the original props along with custom additions (e.g. props.children({ ...props, foo: true }))?

longer-term, maybe it makes sense to merge incoming props or more formally support passing custom props down to props.children()?

Environment

Gatsby version: 1.9.149
Node.js version: 8.9.3
Operating System: macOS 10.12.6

[baseten]

I've just been trying to do this today and I've been finding that the props in my page components aren't being updated to match those being sent from the layout when they change. They seem to remain on the value of the first render. I'm using Gatsby 1.9.149 with React 16.2.0.

I pass them down like so:

// when the value of foo changes it is not propagated down to child components, but remains at value provided during first render
{ props.children( Object.assign( {}, props, { foo: true } ) } 

Gatsby's ComponentRenderer seems to be the culprit, shouldComponentUpdate returns false by default and only checks pageResources, which makes passing dynamic props this way unusable at the moment.

[jeblister]

My solution was to create a class component and pass the props to child component using childContextTypes static object.

Define below context in parent Layout component.

static childContextTypes={
      foo: React.PropTypes.bool
 }

Then populate the value using getChildContext() in Layout class

  getChildContext=()=>{
   return {
   foo: true
  }
  }

Now you can get the value in child component by defining context types like below

static contextTypes={
   foo: React.PropTypes.bool
}

Now you can access the property value in child component using the context like below

let foo= this.context.foo 

[busticated]

@jeblister are you sure that's doing what you think? iirc, context-driven updates end up getting blocked by shouldComponentUpdate so quoting @baseten above...

Gatsby's ComponentRenderer seems to be the culprit, shouldComponentUpdate returns false by default and only checks pageResources, which makes passing dynamic props this way unusable at the moment.

in short, i think what you are doing there is basically the same as props.children( Object.assign( {}, props, { foo: true } ) since in both cases foo is only set once

[jeblister]

@busticated this solution work for me.

I tried the solution of @baseten without result.

[busticated]

@jeblister i put together a demo showing the issue:

https://gist.github.com/busticated/1170897ccabeb566951a209d673b9eb1

TLDR; shouldComponentUpdate() is blocking updates to pages [more info here].

screenshots:

[calcsam]

I'm going to put in a fix for the shouldComponentUpdate on the ComponentRenderer.

Re: the original suggestion of passing along all props by default, a bit wary of that -- what if someone intentionally removed a couple of keys from the Route props dict, then passed that through to children.props()? The original change suggestion would be a breaking change for this use-case, as well as making it difficult (impossible?) to accomplish in a different way.

[baseten]

@calcsam I'm not too familiar with the architecture so forgive me if this won't work, but how about allowing the following:

// this removes the object merging from the layout
{ props.children( { foo: true } ) } 

Then internally the children function merges this object into an explicit property say childProperties of whatever internal props gatsby needs, which you can explicitly test against in shouldComponentUpdate?

Object equality could be an issue here, but not sure if that's the developer's or gatsby's responsibility to take care of

[tomoakley]

I'm trying to pass down the props like { props.children({ ...props, foo: true }) }, but when I try to render it, I get this console error:

Invariant Violation: Objects are not valid as a React child (found: object with keys {content}). If you meant to render a collection of children, use an array instead or wrap the object using createFragment(object) from the React add-ons.

I think it's because it's passing down props.children in the children function, which is then creating a circular dependency? Unsure why others aren't affected by this though.

My use case is, I'm using Contentful to grab some 'pages' (my content type) with various fields like content, title, image, etc. I don't want to have a custom query for each page, I'd rather just have a query in the layout and pass down the data through props into each page that is loaded.