feat(gatsby): extend core theming composition API to be recursive

[ChristopherBiscardi]

Description

Getting this up to reference from other PRs as ongoing work.

• extend the core theming composition API to be recursive, meaning themes can have parents and children.
• add theming to the redux store for usage in plugins like component shadowing
• update component shadowing for multiple themes

Related examples PR, showing the usage of the additions: ChristopherBiscardi/gatsby-theme-examples#13

[ChristopherBiscardi]

Tagging @jbolda @DSchau for visibility since your recent PRs have been theme related.

[DSchau]

This would definitely solve (one of) the issues I was running into in #10786. If there's a JSON structure I can use that contains the known path to themes--that's all I need!

Still want to think about whether it might make sense to offer some mechanism outside of themes (e.g. thinking like a custom component that uses StaticQuery or something), but that's not as crucial and could be a little fringe-y.

[jbolda]

A thought coming off of #10739: I had mentioned I believed that themes shouldn't need to supply a gatsby-config.js. I am going to walk back on that a bit. What if we consider the recursive composability of configs and the component shadowing as two separate pieces. Really, the configs could be useful within plugins as well (and I think you were coming to this as well @DSchau ). The component shadowing really seems to be specifically theme based.

Presuming themes will move into the main plugins array, we need to be able to identify them somehow. What if a theme needs to declare itself as a theme within the config (effectively requiring a config for themes)? Essentially it would be opting into the component shadowing as config composability is could be used by any plugin.

Thoughts?

[ChristopherBiscardi]

Mostly I think this conversation (What are the differences between themes and plugins and should they be defined in the same array) is best left to the time at which we start considering "Themes 1.0". There's plenty that will happen between now and then and IMO it's too early to optimize this. I'm intentionally kicking that conversation down the road as I believe there will be more functionality added to themes. It's far easier to later merge them into the same array if it turns out that's the right path than it is to split them up after we've merged them (which could result in more community turmoil and a longer deprecation process).

[DSchau]

Looks mostly good. I do think the if (themes) will actually break non-themed Gatsby sites so need to address that.

error Plugin webpack-theme-component-shadowing returned an error
  TypeError: Cannot read property 'map' of undefined

[DSchau]

Maybe tweak this to be consecutive await calls?

e.g. I think this is the same effect:

module.exports = async config => {
  const themes = await Promise.mapSeries(
    config.__experimentalThemes,
    async themeSpec => {
      const themeObj = await resolveTheme(themeSpec)
      return processTheme(themeObj)
    }
  )
    .then(arr => {
      const flatThemes = _.flattenDeep(arr)
      debug(flatThemes)
      return flatThemes
    })

  return themes
    .mapSeries(({ themeName, themeConfigObj = {}, themeSpec }) => {
      return {
        ...themeConfigObj,
        plugins: [
          ...(themeConfigObj.plugins || []),
          // theme plugin is last so it's gatsby-node, etc can override it's declared plugins, like a normal site.
          { resolve: themeName, options: themeSpec.options || {} },
        ],
      }
    })
    .reduce(mergeGatsbyConfig, {})
    .then(newConfig => ({
      config: mergeGatsbyConfig(newConfig, config),
      themes,
    }))
}

[ChristopherBiscardi]

I definitely intend to clean this part of the code up. I need to add a bunch of comments too before this gets merged.

[DSchau]

Heads up @ChristopherBiscardi ran prettier on the file that was causing the lint error so you'll have to pull from your remote. 🙃 I think it might have been a line-ending, not sure.

[ChristopherBiscardi]

@DSchau eep ok, thanks for the heads up. I usually rebase instead of merging master in for my personal workflow.

[ChristopherBiscardi]

This is functionally complete.

I want to do the following before merging:

• Deal with formatting/lint/etc
• Comment code and generally make it "easier to understand" for future people

[DSchau]

This is looking great, and I think we can merge. I particularly like tracking on the themes in redux, that's a nice improvement.

One very helpful thing would be getting an e2e test set up for theming, so that we can easily track on regressions in an actual Gatsby app running themes/child-themes/etc.

For now though I think this is ready for a merge.

[ChristopherBiscardi]

@DSchau happy to write up and PR an e2e test. Where's the best place to look for other examples I can copy?

[DSchau]

Published in gatsby@2.0.103 🎉

Thanks @ChristopherBiscardi!