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

Support referencing theme values when defining other theme properties #961

Closed
folz opened this issue May 26, 2020 · 12 comments
Closed

Support referencing theme values when defining other theme properties #961

folz opened this issue May 26, 2020 · 12 comments

Comments

@folz
Copy link
Contributor

folz commented May 26, 2020

Is your feature request related to a problem? Please describe.
When defining theme properties, I sometimes have to copy the same value to two different properties. One common example is when I'm defining raw token names and use-case token names for the same color value:

colors: {
  text: '#333333',
  black: '#333333',

  brand: '#00A846',
  green: {
    500: '#00A846',
  }
}

Describe the solution you'd like
I'd like the ability to reference theme values when defining other theme properties. E.g. (syntax also is just for example, subject to change based on discussion):

colors: {
  text: $ref('colors.black'),
  black: '#333333',

  brand: $ref('colors.green.500'),
  green: {
    500: '#00A846',
  }
}

I have some use cases / restrictions for a theme reference beyond just "matching values":

  • When a referent value changes, any reference also changes.

    E.g. if I make colors.black #000000, I would expect colors.text to also point at #000000

  • I would expect theme-ui to catch any circular references and warn/exit. Either at build time or runtime is fine, but build time is preferred.

    E.g. text: $ref('colors.black'), black: $ref('colors.text') should not enter an infinite loop.

  • It could be nice if theme-ui could optimize away any overhead of a $ref at build time, if it detects that's possible.

    E.g. use --theme-ui-colors-black as the CSS variable for both colors.text and colors.black.

    However, this may not always be what we want, especially if downstream consumers expect they can override --theme-ui-colors-text outside of sx (maybe the team uses CSS modules.)

Describe alternatives you've considered
We could treat theme.js as as compile target and use something like Dhall to write non-duplicated theme rules. That's a lot of overhead to bring into a project if you're not already using Dhall.

I actually have a userland implementation of $ref that is effectively themeGet with a __$ref__ = true property, and have a custom function that keeps applying {css} from @theme-ui/css until no returned value is a $ref. This works, but it's hacky. It'd be nice to have this supported by theme-ui.


This issue is meant to track "Idea: Provide some solution for self-referencing scales in other scale definitions" in #832

@folz
Copy link
Contributor Author

folz commented May 27, 2020

Instead of implementing a $ref equivalent, one thing that might be interesting to explore is inverting the definition to focus on the value, not the token. That is,

colors: {
  '#333333': ['black', 'text'],
  '#00A846': ['green.500', 'brand']
}

This is inspired by how Modulz approaches surfacing theme values in their visual editor:

Screen Shot 2020-05-27 at 2 21 04 PM

We'd sacrifice the ability to define neat scales like {green: {100: ...}}, and we'd need to do some kind of check to make sure that we don't have duplicate token names across values (right now we get that for free because object keys must be unique).

On the other hand, I've come to like how Modulz places emphasis on the actual value, and handles the theme scale aspect under the hood.

@hasparus
Copy link
Member

hasparus commented May 28, 2020

Alternative idea. Possible to typecheck and autocomplete for nice DX. It's close to current derived properties in style objects, so it doesn't introduce any special new syntax.

const theme = {
  colors: {
    primary: '#0022ff",
    success: t => t.colors.primary,
    secondary: t => lighten(0.2, invert(t.colors.primary)),
  }
}

@folz
Copy link
Contributor Author

folz commented May 28, 2020

Yep, I agree. My implementation of $ref is basically themeGet, so in this example

import {themeGet} from 'styled-system'; // I don't think there's an impl in theme-ui
const $ref = path => theme => get(theme, path);

colors = {
  primary: '#0022ff',
  alias1: theme => theme.colors.primary,
  alias2: theme => get(theme, 'colors.primary'),
  alias3: themeGet('colors.primary'),
  alias4: $ref('colors.primary'),
}

all the aliases would all be functionally (ha) equivalent, based on the idea of allowing functions in themes to be called against the theme value in context.

The reason I'm partial to string-based paths is that it constrains more than a generic function, so it seems like we'd be better able to statically analyze the path at build time for optimizations. But a function resolver would work just as well at runtime.

@jxnblk
Copy link
Member

jxnblk commented May 28, 2020

it seems like we'd be better able to statically analyze the path at build time

I like re-using the existing functional syntax, but wrapping that into something like $ref for exactly this reason. If we can statically analyze this with something like a Babel plugin, but also support JSON-serializable formats, I think that's the best of all worlds

@hasparus
Copy link
Member

like we'd be better able to statically analyze the path at build time for optimizations.

I may be a bit allergic to string-based paths, because they can't be typechecked, so no autocomplete possible.

Deriving more scale values from a minimal set of scale values with full power of babel-macro-optimized build-time JavaScript, seems very attractive to me. Maybe too much 😅

This might be relevant to leave here as a note:
https://github.com/styled-components/babel-plugin-polished

@dburles
Copy link
Contributor

dburles commented Jun 5, 2020

Just wondering if there was any real disadvantage to just doing something like this:

const baseColors = {
  black: '#333',
  green: {
    500: '#00A846',
  },
};

const theme = {
  colors: {
    ...baseColors,
    text: baseColors.black,
    brand: baseColors.green.500,
  },
};

@jxnblk
Copy link
Member

jxnblk commented Jun 5, 2020

@dburles I think that's the current recommended way to handle it, but it wouldn't work for JSON-serializable formats, and I think it's been a feature request for years (in Styled System, etc.), so I'm open to exploring a better API for this sort of thing

@dburles
Copy link
Contributor

dburles commented Jun 5, 2020

Not sure I understand completely the intent behind JSON serialising themes, definitely interested if there’s somewhere I can find more context behind it.

@NickChristensen
Copy link

Here's another use-case for this: I want darker shadows in my dark color mode than light.

Because I can't reference defined color values inside shadow, as far as I understand, I'm unable to have the color in my theme's shadow change based on the color mode.

@kbrgl
Copy link

kbrgl commented Aug 4, 2020

I have the same use case as @NickChristensen. I think it'd be nice for the docs to make clear that the theme specification object does not accept functional values, and I’d be happy to send a PR to this effect.

Edit in case anyone needs a workaround for this use case. I'm using gatsby-plugin-theme-ui:

// In src/gatsby-plugin-theme-ui/index.js
export const defaultShadow = theme => `10px 10px 0 0 ${theme.colors.primary}`

And in your component

// Import the shadow
import { defaultShadow } from '../gatsby-plugin-theme-ui'

// Later, when you need to use it:
const MyComponent = () => (
  <div sx={{
    width: 100,
    height: 100,
    boxShadow: defaultShadow
  }}>A Box</div>
)

😄

@atanasster
Copy link
Collaborator

Hi guys,

Great conversation, just came across and put together a babel plugin, where we can enforce theme files to be called ie purple.theme.js to use it. https://github.com/atanasster/babel-plugin-theme-ui

Can you please check if it would be an ok direction for string-based paths (the existing function shortcuts to access theme props already exist).

The project is a very simple conversation starter, just to check the overall expectations. Please also add more test cases for required transformations. The examples are only for colors but it should work for any keys inside the theme.

From this theme definition:
https://github.com/atanasster/babel-plugin-theme-ui/blob/master/tests/fixtures/theme-ui%20parser/code.js

It would transform to the following output:
https://github.com/atanasster/babel-plugin-theme-ui/blob/master/tests/fixtures/theme-ui%20parser/output.js

Of note:

  1. in the following example the lookup will in the current section of the theme (green.30 is inside colors)
colors: {
    green:{
      30: '#00aa00',
    },
    secondary: 'green.30'
  },
  1. in the following example the lookup is in a global scope of the theme colors.primary
  colors: {
    primary: '#ffffff',
  },
  input: {
    bg: 'colors.primary'
  }

@atanasster
Copy link
Collaborator

@hasparus I think we can close this in favor of #1234

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

No branches or pull requests

7 participants