Skip to content
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

fix: Make the Gatsby plugin's theme configurable #362

Merged
merged 1 commit into from
Feb 24, 2020
Merged

fix: Make the Gatsby plugin's theme configurable #362

merged 1 commit into from
Feb 24, 2020

Conversation

trevorblades
Copy link
Contributor

@trevorblades trevorblades commented Jan 29, 2020

Since the src directory was not being published by this plugin, its theme.js file was not shadowable. This was happening because we were transpiling the plugin files and moving them all to the package root before publishing. More info on this subject in this comment.

Since the transpile step is not required since Gatsby runs all dependencies through babel-loader, this PR moves the plugin's gatsby-*.js files to the package root and keeps src/theme.js in place so it can be shadowed again.

Fixes #347
Fixes #425

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8d9f19c:

Sandbox Source
unruffled-sunset-7ptjq Configuration

@trevorblades
Copy link
Contributor Author

trevorblades commented Jan 31, 2020

@alexluong @alexlbr @segunadebayo what are your thoughts on this change? Currently, users of the Gatsby plugin are unable to override the default theme, making it unusable in many cases.

I've put together a reproduction of the error and an example of the fix in action to help understand the problem/solution.

@alexluong
Copy link
Contributor

Hey @trevorblades, sorry for the late response. The reason we transpiled the plugin was some issues in regards to monorepo:

We apologize for the regression. It seems like this PR is just a revert of #265. Any chance you can confirm it will work inside a monorepo?

@trevorblades
Copy link
Contributor Author

trevorblades commented Feb 16, 2020

@alexluong for sure! I've put together an example of the configuration described in gatsbyjs/gatsby#19042. You can check out the full example at https://github.com/trevorblades/gatsby-plugin-chakra-ui-monorepo and run it locally to make sure it works. 👍

git clone https://github.com/trevorblades/gatsby-plugin-chakra-ui-monorepo.git
cd gatsby-plugin-chakra-ui-monorepo
yarn
yarn workspace www start
# everything should be a-ok

In this example I'm using a non-transpiled version of gatsby-plugin-chakra from another git repo within a Gatsby theme in one of the monorepo directories. I have to install from a repo other than this one because NPM expects the package to exist in the root of the repo if you're installing from git. I'm then installing and using the theme within a Gatsby site in a different monorepo directory using Yarn workspaces, as the original issue does.

# install plugin + peer dependencies in theme
yarn workspace gatsby-theme-example add gatsby-plugin-chakra-ui@github:trevorblades/gatsby-plugin-chakra-ui @chakra-ui/core @emotion/core @emotion/styled emotion-theming
# install theme in gatsby site
yarn workspace www add gatsby-theme-example

Let me know if this works for you!

@trevorblades trevorblades changed the title Fix: Make the Gatsby plugin's theme configurable fix: Make the Gatsby plugin's theme configurable Feb 22, 2020
@segunadebayo
Copy link
Member

@trevorblades, I don't know much about gatsby theme that's why I haven't merged it. Sorry for the delay.

I'm relying on the @alexluong to give me the green light. Kindly review @alexluong or anyone familiar with the gatsby theme and I'll merge it.

Thanks

@alexluong
Copy link
Contributor

Hey @trevorblades, thank you for taking the time to create the reproduction.

However, there is some issue with it. Here is the repoduction step on top of your gatsby-plugin-chakra-ui-monorepo:

  1. In www, try to override the default Chakra theme:
// www/src/gatsby-plugin-chakra-ui/theme.js

import { theme } from "@chakra-ui/core";

export default {
  ...theme,
  colors: {
    ...theme.colors,
    blue: {
      ...theme.colors.blue,
      200: "mediumseagreen"
    }
  }
};
  1. In the index page, use a Button:
import React from "react";
import { Button } from "@chakra-ui/core";

export default function Index() {
  return <Button variantColor="blue">Hello</Button>;
}
  1. Try yarn workspace www start: it will behave as expected

  2. Add the build and serve commands and try yarn workspace www build && yarn workspace www serve: in my setup it doesn't work as expected.

Let me know if you can confirm the issue. Thanks for your hard work @trevorblades!

@trevorblades
Copy link
Contributor Author

trevorblades commented Feb 23, 2020

@alexluong this works for me. What yarn version are you working with? I'm on 1.17.3 and I'm not seeing any issues.

Here's a video of my experience going through your repro steps: https://www.youtube.com/watch?v=Ptr3Z9R-S7A

I've also updated my GitHub repo with the theme shadowing and package.json script additions, and hosted the built site on Netlify (notice the mediumseagreen button).

@alexluong
Copy link
Contributor

alexluong commented Feb 23, 2020

Interesting. Lemme take a look into it again.

I'm using yarn 1.21.1.


UPDATE:

So I've looked into the repo again. This is super odd, so I'm not sure what's going on.

What happens is that in the theme, when I override blue.200, the development site works but the production doesn't. When I override blue.500, the reverse happens.

Not sure why that's the behavior, but I can confirm everything else related to Gatsby and shadowing the theme works.

Any thoughts @trevorblades?

@trevorblades
Copy link
Contributor Author

trevorblades commented Feb 24, 2020

@alexluong ok I think I've tracked down the cause of this issue. I've updated my monorepo example to print out the overwritten variable, and color the text with blue.500 like so:

import React, {Fragment} from 'react';
import {Button, Text, useTheme} from '@chakra-ui/core';

export default function Index() {
  const theme = useTheme();
  return (
    <Fragment>
      <Button variantColor="blue">Hello</Button>
      <Text color="blue.500">{theme.colors.blue[500]}</Text>
    </Fragment>
  );
}

Here's what I get in development:

Screen Shot 2020-02-23 at 9 41 16 PM

And in production:

feaea32f349a312a0ad4801be58c580a

In the first image, the button color is not overwritten because the Button component in dark mode uses the 200 variant of the color passed in. However in light mode, the 500 variant is used. This explains the appearance of the button in production, since colorMode doesn't play nicely with SSR at the moment, as described in #349.

For reference, here's how it looks in development when I switch my system color mode. This is the expected behaviour (blue.200 in dark mode, blue.500 in light mode):

e105c3ec8107e424c8f7fe56876e6869

The white flash in the production GIF is what happens when the page loads and the static-rendered page starts in light mode. Due to the bug mentioned above, once the page switches over to dark mode, the button color doesn't get updated appropriately.

One way to get consistent results across development and production in a SSR app like Gatsby would be setting isUsingColorMode to false, unfortunately. One could still wrapRootElement with a <DarkMode> component if they want to force dark mode across the app in server and client renders alike.

import React from 'react';
import {DarkMode} from '@chakra-ui/core';

export const wrapRootElement = ({element}) => <DarkMode>{element}</DarkMode>;

All that being said, I think this issue is outside of the scope of this PR, and will likely need to be addressed separately. What do you think?

@alexluong
Copy link
Contributor

Thanks for doing the investigation, @trevorblades. As I said, besides that, everything else seems right. You can merge and publish the new version @segunadebayo! Thanks.

@segunadebayo segunadebayo merged commit 36dda05 into chakra-ui:master Feb 24, 2020
@trevorblades trevorblades deleted the configurable-gatsby-plugin-theme branch February 24, 2020 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Theme] gatsby-plugin-chakra-ui [gatsby-plugin-chakra-ui]: Not overwriting theme
3 participants