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

Composing Gatsby Sites #8917

Merged
merged 2 commits into from Oct 16, 2018

Conversation

Projects
None yet
6 participants
@ChristopherBiscardi
Copy link
Contributor

ChristopherBiscardi commented Oct 8, 2018

This is the first step towards gatsby themes. It is low level and defines the way multiple gatsby sites compose by defining the way in which gatsby-config's compose. Everything else will build on top of this composition model so it's important to make it extensible and maintainable for the future.

For those that are mathematically inclined, this defines a monoid for the gatsby-config data structure such that (siteA <> siteB) <> siteC === siteA <> (siteB <> siteC). This makes it nice when thinking about sub-theming in the future (imagine a complex ThemeA <> subthemeA <> ThemeB <> subthemeB <> user-site situation)

This method of composition opens the door to themes and sub-themes and allows us to get more user input into how to deal with potentially conflicting artifacts (such as two singleton plugins being defined), test out approaches to generic overriding the rendering of components in user-land, and more.

Themes

A theme is defined as a parameterizable gatsby site. This means that gatsby-config can be a function that accepts configuration from the end user or a subtheme. This is important because in the current state of the world when setting up plugins like gatsby-source-filesystem, we need them to be configured with a __dirname from the user's site (we could have a special __inTheCurrentSite value in the future instead).

In the end-user's site, we declare a "theme" using the __experimentalThemes keyword in gatsby-config. We use this keyword so that people are aware this functionality is experimental and may change without warning. A theme can be configured in the same way plugins are configured (TODO: change [theme, config] syntax to match plugin {resolve:,options} form) so that the userland APIs match up.

// gatsby-config.js
module.exports = {
  __experimentalThemes: [[`blog-theme`, { dir: __dirname }]],
}

The theme then includes a gatsby-config.js which allows it to defined all of the expected fields, such as plugins, and also configure them based on user input. (TODO: looks like gatsby-config.js is ignored in the .gitignore file for the theme package in commit #2)

// blog-theme gatsby-config.js
module.exports = ({ dir }) => ({
  siteMetadata: {},
  plugins: [
    `gatsby-mdx`,
    {
      resolve: `gatsby-source-filesystem`,
      options: {
        name: "blog-posts",
        path: `${dir}/blog-posts/`,
      },
    },
  ],
})

Composing themes

Multiple themes can be used, although there is (intentionally) nothing included in this PR to stop or resolve potential conflicts (for example if a gatsby plugin needs a singleton instance for some reason).

// gatsby-config.js
module.exports = {
  __experimentalThemes: [`blog-theme`, `store-theme`],
}

Themes as plugins

Themes are also included in the plugin list, so they can take advantage of using files such as gatsby-node. When being used as plugins, themes receive the full themeConfig as the options object. As an example, a blog theme could be instantiated multiple times on a site, once for blog posts and once for product reviews.

// gatsby-config.js
module.exports = {
  __experimentalThemes: [
    [`blog-theme`, { baseUrl: '/posts' }},
    [`blog-theme`, { baseUrl: '/reviews' }]
  ],
}

etc

Commits

This PR contains two commits. The first is the actual functionality, the second is a set of examples (a theme defined as an npm package and an example site using said theme). I expect to remove the second commit before merging, but am open to other approaches to keep an example, etc around and develop it further as we progress.

This PR intentionally does not cover:
  • Defining data types in any way different than the current sourcing patterns
  • Any official sub-theming support for overriding components, etc
    • the only way to "override" things right now is to use gatsby lifecycles (ex: on-create-page hooks) to replace the full page component.
    • still technically possible in user-land, planned but not included in core yet

@ChristopherBiscardi ChristopherBiscardi requested a review from KyleAMathews Oct 8, 2018

@ChristopherBiscardi ChristopherBiscardi requested a review from gatsbyjs/core as a code owner Oct 8, 2018

@jlengstorf
Copy link
Member

jlengstorf left a comment

This is super exciting! I have a couple questions that aren't blocking for the merge, so I'm just submitting them as comments.

Show resolved Hide resolved packages/gatsby/src/utils/__tests__/merge-gatsby-config.js
module.exports = (a, b) =>
_.uniq(Object.keys(a).concat(Object.keys(b))).reduce((acc, key) => {
const mergeFn = mergeAlgo[key]
acc[key] = mergeFn ? mergeFn(a[key], b[key]) : b[key] || a[key]

This comment has been minimized.

@jlengstorf

jlengstorf Oct 8, 2018

Member

A comment clarifying this logic might be helpful for future maintainers. Something to the effect of, "If a merge function is not defined, the config option will be overridden."

I say this because it took me a bit to think through this logic and understand how e.g. proxy overrides but plugins merge.

This comment has been minimized.

@ChristopherBiscardi

ChristopherBiscardi Oct 8, 2018

Contributor

I'm open to rewriting this too. Do you think putting the fallback into the list of algorithms would be more approachable? Could combine it with spreading to make it a bit less "compressed".

.reduce((acc, key) => {
    const mergeFn = mergeAlgo[key] || mergeAlgo.fallback
    return {
      ...acc,
      [key]: mergeFn(a[key], b[key])
    }
})
...

{
  fallback: (a,b) => b || a.
  ...
}

This comment has been minimized.

@jlengstorf

jlengstorf Oct 8, 2018

Member

That could help for sure!

Perhaps naming would help in this case, too? E.g. mergedConfig vs. acc, overrideOption vs. fallback, mergeConfigOptions vs. mergeFn?

We're now crossing over into Personal Taste™, but my experience has been that Future Jason tends to curse Past Jason far less for verbose variable names. 😅

This comment has been minimized.

@ChristopherBiscardi

ChristopherBiscardi Oct 9, 2018

Contributor

I tried to simplify this, although I think I can pull too much from the context (and from having written it) to make it any easier without more feedback.

ChristopherBiscardi added some commits Oct 8, 2018

Composing gatsby sites
This is the first step towards gatsby themes. It is low level and
defines the way multiple gatsby sites compose by defining the way in
which gatsby-config's compose.

For those that are mathematically inclined, this defines a monoid for
the gatsby-config data structure such that `(siteA <> siteB) <> siteC
=== siteA <> (siteB <> siteC)`.

This method of composition opens the door to themes and sub-themes and
allows us to get more input into how to deal with potentially
conflicting artifacts (such as two singleton plugins being defined).

A theme is defined as a parameterizable gatsby site. This means that
gatsby-config can be a function that accepts configuration from the
end user or a subtheme.
PR Feedback
* Try to make the merge code more approachable for beginners, etc.
* Add test for duplicate plugin values

@ChristopherBiscardi ChristopherBiscardi force-pushed the ChristopherBiscardi:gatsby-config-composition branch from 2ad7059 to aba5452 Oct 9, 2018

@ChristopherBiscardi

This comment has been minimized.

Copy link
Contributor

ChristopherBiscardi commented Oct 9, 2018

I also removed the example code, since some package updates due to the examples were causing test snapshot failures. This can be squashed and merged "as is"

@jbolda

This comment has been minimized.

Copy link
Contributor

jbolda commented Oct 9, 2018

Perhaps this has some background somewhere that I'm unaware of, @ChristopherBiscardi, but what is the value in having a separate keyword as opposed to solely putting under plugins?

@ChristopherBiscardi

This comment has been minimized.

Copy link
Contributor

ChristopherBiscardi commented Oct 9, 2018

@jbolda Since this is an experimental feature, combining the keys could break anyone who happens to have a gatby-config file in those locations and immediately forces the entire user base into using it on release. It would be easy to unknowingly opt-in to experimental functionality just by adding a gatsby-config in the wrong place (where as before it would do nothing, now it would do something). The additional key is acting as an opt-in mechanism that also doesn't require the user to change their build config to test it. A flag via env var could be another solution, but requires build config changes to enable the functionality. The way it's handled right now, the code doesn't run if you don't explicitly opt-in, so everything functions as expected. I'm not against combining them in the future, but it seems like having them separate is the right move as we iterate on the functionality.

@jbolda

This comment has been minimized.

Copy link
Contributor

jbolda commented Oct 10, 2018

Oh! That makes a lot of sense. I was thinking the long-term plan was intended to keep it as its own keyword. This definitely should be opt-in for the time being. 👍

Thanks for taking care of this! I have been looking forward to this functionality for some time.

@DSchau DSchau referenced this pull request Oct 12, 2018

Closed

remove themes doc page #9058

@calcsam
Copy link
Member

calcsam left a comment

This is amazing @ChristopherBiscardi -- we're really excited about this.

@calcsam calcsam merged commit 26f7cc7 into gatsbyjs:master Oct 16, 2018

11 checks passed

ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsbygram Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_runtime Your tests passed on CircleCI!
Details
ci/circleci: integration_tests Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node10 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node6 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node8 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@ghardin137

This comment has been minimized.

Copy link
Contributor

ghardin137 commented Oct 19, 2018

I know this has already been merged, but this actually causes problems with the tutorial repo. gatsby-starter-hello-world does not contain a gatsby-config.js file and this PR isn't checking if the config file exists before trying to use it.

@DSchau

This comment has been minimized.

Copy link
Contributor

DSchau commented Oct 19, 2018

@ghardin137 that's great feedback, and we'll want to get that fixed up. Thanks!

ghardin137 added a commit to ghardin137/gatsby that referenced this pull request Oct 19, 2018

pieh added a commit that referenced this pull request Oct 19, 2018

jedrichards added a commit to jedrichards/gatsby that referenced this pull request Nov 1, 2018

Composing Gatsby Sites (gatsbyjs#8917)
This is the first step towards gatsby themes. It is low level and defines the way multiple gatsby sites compose by defining the way in which gatsby-config's compose. Everything else will build on top of this composition model so it's important to make it extensible and maintainable for the future.

For those that are mathematically inclined, this defines a monoid for the gatsby-config data structure such that `(siteA <> siteB) <> siteC === siteA <> (siteB <> siteC)`. This makes it nice when thinking about sub-theming in the future (imagine a complex `ThemeA <> subthemeA <> ThemeB <> subthemeB <> user-site` situation)

This method of composition opens the door to themes and sub-themes and allows us to get more user input into how to deal with potentially conflicting artifacts (such as two singleton plugins being defined), test out approaches to generic overriding the rendering of components in user-land, and more.

## Themes

A theme is defined as a parameterizable gatsby site. This means that gatsby-config can be a function that accepts configuration from the end user or a subtheme. This is important because in the current state of the world when setting up plugins like `gatsby-source-filesystem`, we need them to be configured with a `__dirname` from the user's site (we could have a special `__inTheCurrentSite` value in the future instead).

In the end-user's site, we declare a "theme" using the `__experimentalThemes` keyword in gatsby-config. We use this keyword so that people are aware this functionality is experimental and may change without warning. A theme can be configured in the same way plugins are configured (TODO: change `[theme, config]` syntax to match plugin `{resolve:,options}` form) so that the userland APIs match up.

```js
// gatsby-config.js
module.exports = {
  __experimentalThemes: [[`blog-theme`, { dir: __dirname }]],
}
```

The theme then includes a gatsby-config.js which allows it to defined all of the expected fields, such as plugins, and also configure them based on user input. (TODO: looks like gatsby-config.js is ignored in the .gitignore file for the theme package in commit gatsbyjs#2)

```js
// blog-theme gatsby-config.js
module.exports = ({ dir }) => ({
  siteMetadata: {},
  plugins: [
    `gatsby-mdx`,
    {
      resolve: `gatsby-source-filesystem`,
      options: {
        name: "blog-posts",
        path: `${dir}/blog-posts/`,
      },
    },
  ],
})
```

### Composing themes

Multiple themes can be used, although there is (intentionally) nothing included in this PR to stop or resolve potential conflicts (for example if a gatsby plugin needs a singleton instance for some reason).

```js
// gatsby-config.js
module.exports = {
  __experimentalThemes: [`blog-theme`, `store-theme`],
}
```

### Themes as plugins

Themes are also included in the plugin list, so they can take advantage of using files such as `gatsby-node`. When being used as plugins, themes receive the full themeConfig as the options object. As an example, a blog theme could be instantiated multiple times on a site, once for blog posts and once for product reviews.

```js
// gatsby-config.js
module.exports = {
  __experimentalThemes: [
    [`blog-theme`, { baseUrl: '/posts' }},
    [`blog-theme`, { baseUrl: '/reviews' }]
  ],
}
```

# etc

##### Commits

This PR contains two commits. The first is the actual functionality, the second is a set of examples (a theme defined as an npm package and an example site using said theme). I expect to remove the second commit before merging, but am open to other approaches to keep an example, etc around and develop it further as we progress.

##### This PR intentionally does not cover:

- Defining data types in any way different than the current sourcing patterns
- Any official sub-theming support for overriding components, etc
  * the only way to "override" things right now is to use gatsby lifecycles (ex: on-create-page hooks) to replace the full page component.
  * still technically possible in user-land, planned but not included in core yet

jedrichards added a commit to jedrichards/gatsby that referenced this pull request Nov 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment