[babel-plugin] Support nested objects in defineConsts

[j-malt]

What changed / motivation ?

Our design system uses three-tiered tokens with component specific tokens like $designSystem-component-button-primary-background-default and $designSystem-component-button-primary-background-hovered. This structure lends itself naturally to a nested theme object:

{
    button: {
        primary: {
            background: {
                default: '#00FF00',
                hovered: '#0000FF',
            },
            borderRadius: {
                 default: '8px'
            }
        },
        secondary: { /* ... */ }
    },
    input: {
        fill: {
            default: '#FFFFFF',
        },
        border: { /* ... */ }
    }
    // ...
}

We use defineConsts to implement this theme object (defineVars doesn't work for us, certain product requirements mean that we need the values of our theme object to be CSS variable references that are defined in an external file).

Currently there is no good way to implement a structure like this with defineConsts. Long camel case keys is an option, but has awkward Intellisense/auto-complete support. Splitting up the definitions into many top-level defineConsts calls (buttonColours, buttonBorderRadius, etc.) is the closest we can get, but is annoying in a few ways:

• Requires many top-level defineConst definitions (approx. (# of components) * (# of properties))
• Reduces discoverability of tokens, as you need to know to which top-level const object to import
• Long/verbose import statements

This PR adds support for arbitrarily nested objects in defineConsts. That is, the following is now valid StyleX:

import * as stylex from '@stylexjs/stylex`

const colors = stylex.defineConsts({
    button: {
        background: { 
            primary: '#FFFFFF'
        }
    }
})

const buttonStyles = stylex.create({
    backgroundColor: colors.button.background.primary
});

Additional Context

Fixture tests were added. I made a small change to the defineConsts documentation to suggest that the objects can be nested. See comments for some notes on the code.

Pre-flight checklist

• I have read the contributing guidelines
  Contribution Guidelines
• Performed a self-review of my code

[j-malt]

A previous iteration of this PR made a change directly to the proxy to "build up" nested accesses overtime, and then had this AST traversal extract out the value once we were at the terminal/outermost member access. This worked well for any code which accesses the proxy via this traversal, but broke for code that accessed it in other ways, specifically variables here:

stylex/packages/@stylexjs/babel-plugin/src/visitors/stylex-create-theme.js

Lines 154 to 162 in d8053a2

	if (
	typeof variables.__varGroupHash__ !== 'string' ||
	variables.__varGroupHash__ === ''
	) {
	throw callExpressionPath.buildCodeFrameError(
	'Can only override variables theme created with defineVars().',
	SyntaxError,
	);
	}

And themeVars[key] here

stylex/packages/@stylexjs/babel-plugin/src/shared/stylex-create-theme.js

Line 47 in d8053a2

const nameHash = themeVars[key].slice(6, -1);

A simple way to fix this would've been to special case these accesses or to make them aware of the proxy. Neither of these seem like great solutions as they would leak the implementation details of this evaluate loop throughout the codebase.

My solution here is to have Babel to traverse the AST as normal, but once we are at the terminal/outermost access of a member expression, extract and join the full key path as a dotted string and pass that to the proxy. This keeps the proxy behaving as normal but means that dotted accesses work. I can add some benchmarks in another PR if there are concerns about build performance here.

[j-malt]

I added a quick benchmark in #1312 and updated the benchmark in this PR to use nested keys as well. This change seems to be insignificant on the "complex create" benchmark.

[j-malt]

I couldn't come up with a great way to get around conflicting keys like this and it feels like a pattern to avoid anyways.

[j-malt]

Is there anything that would be helpful to get this change moved along? This would be super valuable for us internally.

[nmn]

I'm still going through the code changes, but I want to lay out my thinking here before leaving any code comments:

Why I'm generally against this

My primary opposition to this API change is that defineConsts is implemented almost like a special-case of defineVars. defineVars does not and can not support nested values like this PR implements.

So, while, yes, it is possible to extend defineConsts to support this API, it feels wrong to have the two APIs diverge like that.

What I think we should do instead

One API that we have been discussing for a few weeks is something called stylex.env which would let you define any arbitrary set of constants in the styleX babel config which would then be available on stylex.env. Think of this as the StyleX interpretation of process.env.

In many ways, stylex.env would be even more powerful than this extension of defineConsts because while defineConsts can only ever be used as individual values, stylex.env would let you use entire objects. This is because stylex.env would be substituted before the stylex.create (etc.) are compiled, while defineConsts depends on CSS variables that are then replaced with values in the final CSS file generation.

Trade-offs

I would argue that stylex.env is a strictly more powerful system for constants with one big downside: cache-invalidation

Since stylex.env is dependent on the configuration of the Babel plugin, any time a constant is changed, it would mean invalidating the cache for the compilation of all JS files, this is not the case with defineConsts or any other StyleX API where the result of compiling each JS/TS file is cacheable as long as the config or the file contents don't change.

Another advantage of stylex.env is that it will lead to overall smaller CSS. With defineConsts, if the same style is defined twice, once when using a const and another with the actual value inlined, it will result in two separate CSS rules with different classNames. This will not be the case with stylex.env.

---

So with all of that said, I would like to understand if a stylex.env API would also work for your needs, because I think we can fast-track that instead extending the defineConsts API.

If you have a strong argument for why it would be more helpful to extend defineConsts instead, please make your case as well!

[mellyeliu]

I have a few thoughts on this.

1. If we allow nested objects for defineConsts, then we might need to rethink and rewrite defineVars and createTheme. Allowing nesting for one API but not the others may get confusing. Nested defineVars calls would probably encourage larger and more complex VarGroups that would make the createTheme override feel more clunky.

2. On stylex.env: I've been thinking of it as more of a middle layer for users or developers to add shareable functions that would otherwise require entire pre-StyleX plugins. It doesn't feel as useable for something high-churn like design system tokens because (1) it sits at an awkward place in the babel config and (2) you're be invalidating the cache with each change. That adds just enough friction for me to think its best use is for infra eng rolling out high-level functions across a codebase.

If we want to try out this nested defineConsts API I'd prefer to keep it behind a config. Or an experimental API to battle test things a bit.

[j-malt]

Thanks both for your thoughts! We chatted a bit elsewhere, but just to summarize:

• As @mellyeliu said, having design-tokens or other high-churn settings located in the Babel config would be pretty awkward for us, both organizationally¹ and conceptually².
• Design tokens are pretty high-churn, and having them in stylex.env would likely end up invalidating our build cache very frequently, which makes this a difficult sell.

It sounds like the best path forward will be to:

1. Close this PR, and keep defineConsts/defineVars as flat objects for now.
2. Ship a set of unstable variants of defineVars/defineConsts/createTheme that allow for nesting
3. Eventually consider shipping a breaking change to defineVars & defineConsts to allow them to support nesting and kill off the unstable APIs.

For now, I'll close this PR out. Thanks again!

Footnotes

1. We maintain a central Babel config that's shared across every package and is located pretty deep in our infra. Having design tokens located there feels awkward for design engineers who typically do not work in that part of our infrastructure. ↩

2. For us, it's helpful to be able to think of design tokens as a dependency like any other. Being able to have packages express dependencies on a set of tokens means it's easier to understand/restrict the usages of tokens, which is helpful both for design system teams and also for infra teams who need to handle migrations. This is also helpful for teams who maintain multiple brands, you can restrict a given brands design tokens to only their branded packages simply by restricting dependencies on that theme. ↩