createSprinkles with multiple collections that have the same property throws an error

[moroshko]

Describe the bug

Passing multiple collections of properties to createSprinkles that share the same property (backgroundColor in this case) results in an error:

sprinkles.css.ts

import { defineProperties, createSprinkles } from "@vanilla-extract/sprinkles";

const responsiveProperties = defineProperties({
  conditions: {
    mobile: {},
    tablet: { "@media": "(min-width: 768px)" }
  },
  defaultCondition: "mobile",
  properties: {
    backgroundColor: ["red", "blue"]
  }
});

const stateProperties = defineProperties({
  conditions: {
    default: {},
    focus: { selector: "&:focus" }
  },
  defaultCondition: "default",
  properties: {
    backgroundColor: ["red", "blue"]
  }
});

export const sprinkles = createSprinkles(responsiveProperties, stateProperties);

App.css.ts

import { sprinkles } from "./sprinkles.css";

export const container = sprinkles({
  // backgroundColor: { default: "red", focus: "blue" } // works, but TypeScript complains
  backgroundColor: { mobile: "red", tablet: "blue" } // doesn't work, runtime error
});

ERROR in ./src/App.css.ts
Module build failed (from ./node_modules/@vanilla-extract/webpack-plugin/loader/dist/vanilla-extract-webpack-plugin-loader.cjs.js):
NonErrorEmittedError: (Emitted value instead of an instance of Error) SprinklesError: "backgroundColor" has no condition named "mobile". Possible values are "default", "focus"
    at processResult (/sandbox/node_modules/webpack/lib/NormalModule.js:673:12)
    at /sandbox/node_modules/webpack/lib/NormalModule.js:778:5
    at /sandbox/node_modules/loader-runner/lib/LoaderRunner.js:399:11
    at /sandbox/node_modules/loader-runner/lib/LoaderRunner.js:195:20
    at context.callback (/sandbox/node_modules/loader-runner/lib/LoaderRunner.js:124:13)
    at /sandbox/node_modules/@vanilla-extract/webpack-plugin/loader/dist/vanilla-extract-webpack-plugin-loader.cjs.dev.js:77:5
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

Link to reproduction

CodeSandbox

System Info

Output of npx envinfo --system --npmPackages @vanilla-extract/css,@vanilla-extract/webpack-plugin,@vanilla-extract/esbuild-plugin,@vanilla-extract/vite-plugin,@vanilla-extract/sprinkles,webpack,esbuild,vite --binaries --browsers:

  System:
    OS: Linux 5.4 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (16) x64 Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
    Memory: 8.49 GB / 62.72 GB
    Container: Yes
    Shell: 5.0.3 - /bin/bash
  Binaries:
    Node: 14.18.1 - ~/.nvm/versions/node/v14.18.1/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v14.18.1/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.18.1/bin/npm
  npmPackages:
    @vanilla-extract/css: 1.6.8 => 1.6.8
    @vanilla-extract/sprinkles: 1.3.3 => 1.3.3
    @vanilla-extract/webpack-plugin: ^2.1.4 => 2.1.4
    webpack: ^5.36.1 => 5.36.1

[graup]

What is the expected behavior?

I recommend to not organize collections by state but by styles (e.g. put everything related to backgroundColor in the same collection).

[moroshko]

@graup The expected behavior is for the styles to be generated and not throw an error.

put everything related to backgroundColor in the same collection

Having one collection per property would result in a lot of collections! I'm wondering what's the best practice here.

@mattcompiles @michaeltaranto @markdalgleish What would you recommend?

[nayaabkhan]

I came here with the same expectation. Happy to see this issue.

Maybe one of our use-cases could help explain why. We're classifying colors based on a few traits:

1. Tonality: Positive, negative, cautionary, neutral, and informational.
2. Action: Primary, secondary, and destructive.

We expected to generate a separate set of atoms for backgroundColor and color using tonality without any conditions. While for actions, we're expecting to generate separate atoms with conditional states like hover, active, and disabled.

[mattcompiles]

Not sure how to solve this one off the top of my head. Seems like there's multiple use-cases.

@moroshko's original issue where it's potentially terser/more readable to allow splitting the same property value between different property groups. While it may not be ideal I'd be suggesting @graup's advice and grouping by style.

However, I think @nayaabkhan raises an interesting point around having different property values with different conditional settings. This is something we can't do right now. You may generate some more CSS than needed but you could just merge all your conditionals and create them all for all your colors. Once again this is not ideal, but unless you have a very large list of colors it may not be a huge issue.

Getting this style of config to work may be quite hard from a types perspective. Also not sure how default styles would work here as you'd likely generate multiple default styles that are the same unless you're very careful.

A short-term solution may be to throw a clear error if clashing property definitions are detected.