Skip to content

Add first class support for extending the default theme #655

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

Merged
merged 6 commits into from
Feb 14, 2019

Conversation

adamwathan
Copy link
Member

One of the changes we are making for 1.0 is to start encouraging people to only replace/extend what they need to change in the config file, rather than scaffolding every single value and giving the user complete ownership by default.

A challenge with this approach is that it's not quite as simple to just add a few new values to your config (or more aptly your theme in 1.0 terms).

Previously all of the default values would already be present, and you could just tack on any extra ones you wanted to add:

  // 0.x config format

  module.exports = {
    // ...
    borderWidths: {
      default: '1px',
      '0': '0',
      '2': '2px',
      '4': '4px',
      '8': '8px',
+     '10': '10px',
+     '12': '12px',
    },
  }

But with the approach we are recommending for 1.0, there would be no borderWidths key in your theme by default, and to add the two new values you'd also have to re-add the existing values.

  // 1.x config format

  module.exports = {
    // ...
    theme: {
+     borderWidths: {
+       default: '1px',
+       '0': '0',
+       '2': '2px',
+       '4': '4px',
+       '8': '8px',
+       '10': '10px',
+       '12': '12px',
+     },
    }
  }

My original thinking for working around this was just adding a new export (something like tailwindcss/defaultTheme) that users could import and spread into their theme to extend things, for example:

+ const defaultTheme = require('tailwindcss/defaultTheme')

  module.exports = {
    // ...
    theme: {
+     borderWidths: {
+       ...defaultTheme.borderWidths,
+       '10': '10px',
+       '12': '12px',
+     },
    }
  }

...but I realized this would break down in combination with the theme-values-as-closures support added in #645 (you can't spread in an object that hasn't even been fully evaluated yet).

So instead, the best idea I could come up with is a new extend key inside the theme section that will properly handle all of this stuff for you.

It looks like this:

  module.exports = {
    // ...
    theme: {
+     extend: {
+       borderWidths: {
+         '10': '10px',
+         '12': '12px',
+       },
+     }
    }
  }

By handling this in a first-class way we can intelligently handle the closure situations to give you intuitive results that make perfect sense but would be almost impossible to implement ergonomically in userland.

For example, this config file adds a few custom colors to the overall color palette, and adds a new background color, with the background colors still inheriting their base values from the user-extended color palette:

module.exports = {
  theme: {
    extend: {
      colors: {
        cyan: 'cyan',
        magenta: 'magenta',
      },
      backgroundColors: {
        customBackground: '#bada55',
      },
    },
  },
}

The result here would be that you'd get all of the default background color utilities (bg-blue, bg-blue-light, etc.), as well as bg-cyan and bg-magenta from the extended color palette, and bg-customBackground for the color added only to backgroundColors. Doing this in user-land with the defaultTheme export and trying to spread stuff in yourself is not really even possible without extracting some variables to avoid duplication.

With this PR, a user theme would end up being broken into two sections: complete replacements, and extensions.

I expect in the real world it would usually look something like this:

module.exports = {
  theme: {

    // Completely replaced values:
    colors: {
      // User's custom color palette
    },
    fonts: {
      // User's custom font stack
    },

    // Values that extend the defaults with a few extras:
    extend: {
      shadows: {
        xl: '0 25px 40px 0 rgba(0,0,0,0.11), 0 10px 20px 0 rgba(0,0,0,0.05)',
      },
      opacity: {
        '60': '.6',
      }
    },
  },
}

'20': '.2',
'40': '.4',
},
height: theme => theme.width,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you intentionally setting the theme.width as the height value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, test is to verify that if you have a closure value in your user config that you can still extend that same value and have the extensions add to the user overrides 👍

}

function mergeExtensions(theme) {
return mergeWith({}, without(theme, 'extend'), theme.extend, (_, extensions, key) => {
Copy link

@ramon-villain ramon-villain Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not you get the same result spreading the parameter?

function mergeExtensions({extend, ...theme}) {
  return mergeWith({}, theme, extend, (_, extensions, key) => {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, so much better, thanks! ❤️

@RoyalIcing
Copy link

RoyalIcing commented Feb 14, 2019

This looks really great! I can definitely see this an improvement over the current ‘create an entire copy’ approach.

One drawback I can see is for TypeScript, it can infer the values when something is splatted ... in, but it won’t be able to see the original values with this, only the new ones.

Basically with TypeScript, you can create a type from an object’s keys. So if you change the object, then what is allowed by that type would also change. But here it would only see the keys listed under extend.

e.g.

// Import the actual tailwind.js config
import Tailwind from "../tailwind";

export type BackgroundColor = keyof typeof Tailwind.backgroundColors;

Something more advanced might by possible where it is able to detect the extend. I can look into this.

@SjorsO
Copy link
Contributor

SjorsO commented Feb 14, 2019

module.exports = {
  theme: {
    extend: {
      fonts: {
        sans: [
          'Open Sans',
        ],
    },
  },
}

Would this work if i want to prepend a font to one of the font stacks? Or would this append it instead?

@adamwathan
Copy link
Member Author

@BurntCaramel Yeah interesting thought, I don't have any experience with Typescript or any real desire to use it so I don't plan to change things drastically to accommodate it. We could probably add some way to export the user's flattened config/theme though so that could be used as a type reference?

@SjorsO Right now that would actually completely replace the sans font family because extensions aren't deeply merged (just added a test for this). I considered added special handling to make it prepend fonts but I think special handling for different keys is a dark road to go down 😕

@garygreen
Copy link

For people like me who like complete control over the classes that are generated, would this approach be anymore difficult? I actually like that the tailwind config is almost like documentation for your app, but I understand your trying to encourage referring to the official documentation rather than your own config. I suspect with this approach, you'll need to refer to both the documentation and config for any possible extensions to get the full picture.

With upgrades, it'll definitely be easier as you don't need to copy and paste new/adjusted values, but I do worry tailwind will slowly keep on creeping up in size without people really knowing why...

@SjorsO
Copy link
Contributor

SjorsO commented Feb 14, 2019

@adamwathan I agree that adding special cases is not a good idea. Adding a paragraph in the documentation about how to prepend a font to a font stack would probably be a better solution.

@adamwathan
Copy link
Member Author

@garygreen:

For people like me who like complete control over the classes that are generated, would this approach be anymore difficult?

Nah won't be any more difficult, you still have just as much control it's just that we won't generate every value and stuff them in your config file for you by default. You will still be able to get a fully-fleshed out config file if you like by doing something like tailwind eject or tailwind init --verbose or something.

I suspect with this approach, you'll need to refer to both the documentation and config for any possible extensions to get the full picture.

Yeah I initially was worried about this being annoying but then I realized it's already true even with Tailwind 0.x, because so many of the classes have no configuration. So if you want to know what all of the overflow related classes are, you have to go to the docs anyways.

With upgrades, it'll definitely be easier as you don't need to copy and paste new/adjusted values, but I do worry tailwind will slowly keep on creeping up in size without people really knowing why...

This actually already happens, too. If we add a new module to Tailwind (like say order for flex order utilities), and you don't have an order key in your config because your config was generated prior to that feature existing, when you upgrade Tailwind will automatically generate those new classes using the default configuration. It's not until you add an order key to your config that the defaults would get overridden, so the behavior isn't actually changing as far as that goes.

@adamwathan adamwathan merged commit 86fe81b into next Feb 14, 2019
@RoyalIcing
Copy link

Yeah interesting thought, I don't have any experience with Typescript or any real desire to use it so I don't plan to change things drastically to accommodate it. We could probably add some way to export the user's flattened config/theme though so that could be used as a type reference?

@adamwathan Yes that sounds like a good solution, and could have uses outside of just TypeScript too.

So if you want to know what all of the overflow related classes are, you have to go to the docs anyways.

One of these uses could be to use the flattened config to generate your own custom documentation! I think would be amazing to say here is my configuration, now generate my custom version of the tailwindcss.com docs. I’m happy to contribute here!

CvX added a commit to CvX/tailwindcss that referenced this pull request Mar 17, 2019
CvX added a commit to CvX/tailwindcss that referenced this pull request Mar 17, 2019
@adamwathan adamwathan deleted the extend-theme branch May 13, 2019 14:00
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.

5 participants