tokens-bruecke / figma-plugin

TokensBrücke is a Figma plugin that converts Figma variables into design-tokens JSON.
MIT License
39 stars 7 forks source link

$meta is not valid DTCG #13

Closed james-nash closed 4 months ago

james-nash commented 5 months ago

I've been using Tokens Brücke to export Figma variables as a DTCG file to then use with Cobalt UI. It works really well - especially since you've adopted the same modes extension that Cobalt UI supports. That's made my life a lot easier!

However, there is one minor snag: Tokens Brücke adds a top-level $meta property to the output with some info about the export settings and creation time. The trouble is that's not valid DTCG as the $ prefix is reserved for format properties and $meta is not something that exists in the current spec draft. Cobalt's parser is following that rule and therefore throws an error because of the $meta property.

While it's easy to work around - just remove that property - it would be nice if I didn't have to and things "just worked". I therefore propose moving that metadata into an extension (the spec allows $extensions on groups, so this would be fine). Perhaps something like this:

{
  // ... design token data

  "$extensions": {
    "tokens-bruecke.meta": {
      // your metadata goes here
    }
  }
}

Cobalt and (hopefully) other tools that can parse DTCG files won't have any issues then.

If you like, I can open a PR to implement this change (looks like it's just one line that would need changing).

dev-nicolaos commented 5 months ago

I agree that using a name-spaced object on the official $extensions property would be a better place to store the meta data.

@james-nash what version of Cobalt are you using? We're currently using the following versions with this plugin and get a warning but no error. Cobalt successfully transforms the tokens.

  "devDependencies": {
    "@cobalt-ui/cli": "^1.6.2",
    "@cobalt-ui/plugin-css": "^1.7.0",
    "@cobalt-ui/plugin-js": "^1.4.3"
  }
james-nash commented 5 months ago

I'm using the same versions. Just checked and, you're right, it's just a warning not an error. (I had another, unrelated issue in my file that was causing an actual error, so mistakenly thought $meta was triggering that)

Nonetheless, I'd still advocate for moving this to an extension for better compliance. Other DTCG parsers might be less forgiving.

dev-nicolaos commented 4 months ago

@PavelLaptev I could probably put together a PR for this if you'd like. Should this be considered a breaking change (major version release) or a bug fix (patch version release)? I can see an argument for both.

PavelLaptev commented 4 months ago

Hi @dev-nicolaos sure if you have time to do so, I would like to check your PR. Let's do it as a minor version, since the only person who's using the meta is me 😁

dev-nicolaos commented 4 months ago

Let's do it as a minor version, since the only person who's using the meta is me 😁

@PavelLaptev since this packages appears to be using semantic versioning, I'd recommend not using a minor version since this change won't introduce any new features. Either the previous behavior is considered a bug and this change is a fix (patch), or we are making a breaking change to an API (major).

james-nash commented 4 months ago

Thanks so much for fixing this!