Treat file path as prefix for property path

[elliotdickison]

Proposal

Include the file path to a property (relative to the source dir provided to style-dictionary) as a prefix to the property path. For example this file:
core/color.json

{
  "red": { "value": "#F00" }
}

would result in a property with the path: ["core", "color", "red"] .

This would be a breaking change.

Reasoning

Currently the benefits of splitting properties into multiple files are limited since property keys still have to be specified in full in each file, and the keys in a file may not actually match the path of a file. This proposed change would allow larger projects to take advantage of nested folders and multiple property files to significantly simplify the json required and ensure consistency between file paths and property keys.

Background

While style-dictionary allows you to split properties into multiple files/folders, this currently has no effect on the way you key properties within the files. For example, splitting all.json into size.json and color.json still requires you to key properties by size and color:

all.json

{
  "size": {
    "small": { "value": "4px" }
  },
  "color": {
    "red": { "value": "#F00" }
  }
}

size.json

{
  "size": {
    "small": { "value": "4px" }
  },
}

color.json

{
  "color": {
    "red": { "value": "#F00" }
  }
}

In our project I found that this caused significant redundancy in our property files. For example I had a file base/icon/checkmark.json that looked something like this:

{
  "base": {
    "icon": {
      "checkmark": {
        "url": "<cdn url here>"
      }
    }
  }
}

Multiplied across dozens of property files nested in multiple folders, this created significant redundancy and opportunities for typos in our dictionary files. I considered two solutions to this problem: use fewer files or pre-process our files and automatically generate the needed keys from the file path. I really like the ability to use files and folders to organize properties (we have 400+ properties and counting) and I didn't want to start merging them back into larger json files. I went the route of preprocessing our property files and generating json keys from file paths, and it's been working very well. We now have a setup where the following file:
base/icon/checkmark.json

{
  "url": "<cdn url here>"
}

is preprocessed so that the actual property path for the url property is ["base", "icon", "checkmark", "url"]. This has simplified our json and allows us to quickly understand the path of a property by looking at the directory structure.

[chazzmoney]

Like this a lot. There is something that is weird / bothers me about how deep merge loses all context of where things are coming from. I have a similar feeling about 'includes' vs 'source'.

A few thoughts that may have some relevance:

• Backwards compatibility should probably be retained so that we don't break anyone's project.
• Backwards compatibility sucks, because it means that the default would still be the old behavior.
• In the thinking around standardizing the CTIs for atoms and mapping those to component CTI (or theme CTIs or brand CTIs or whatever), this could make life a lot easier
• This could provide a nice structure for building a SD from an import
• Does this mean that we would need to specify a directory as the 'source' instead of files/wildcard file paths? How do we decide how deep into the directory structure to go?
• When we generate the merged property values, should we save the path info in something beyond the key? (as an additional attribute?)

I know that all this stuff wasn't exactly what you were suggesting, but I wanted to get the thinking out about how best to integrate the concept.

[elliotdickison]

Re: backwards compatibility, I'm thinking this change would only really make sense as part of a major version bump. I think the added complexity (in terms of API/usage/docs) could make the opt-in strategy not worth it.

Re: source path vs globs, I didn't even notice that the current config is glob-based. That does make things more complicated. It actually makes me realize that this change might have a much wider impact than I thought in terms of how style-dictionary could be used. Currently the globs would allow you to mix property files in with any other files, making style-dictionary something that could be integrated into an existing project (sort of like the way storybook works for react projects). I'm not sure if people actually do that, but it'd be theoretically easy. My proposed change was based on the assumption that dictionaries are kind of standalone entities where you have all of your property files in a dedicated folder (or repo), apart from any project-specific files, and I think it would work best in that scenario.

Our project is definitely going the dictionary-as-a-standalone-thing route, but before pursuing this change it might be worth it to find out how others are using it.

[dbanksdesign]

TBH about path globs, I don't know if it is 100% necessary. I've actually never used it's potential, all my style dictionaries use "properties/**/*.json" and you are right, I always structure the files and directories based on the JSON structure. I think this proposal is most likely the best path forward, and should be part of a major version bump. Maybe I'll start a project with all the proposals for Style Dictionary 3.0 and we could start working on them.

I do want to think about this a bit more. One of the core principles in my mind is to not force users of Style Dictionary into a particular structure. I like seeing how @elliotdickison built their style dictionary in a way I had 0 idea would be possible or happen. I want to keep that spirit alive, so I want to make sure this change does not hinder people.

[chazzmoney]

We should finish up this conversation and decide whether to add to 3.0.

[dbanksdesign]

I should note that we can do this today with node module property files. An example is in this demo repo:
https://github.com/dbanksdesign/style-dictionary-node

You can see it in some of the property files:

module.exports = {
  core: require('./core'),
  brand: require('./brand'),
  font: require('./font'),
  background: require('./background')
}

The thing to be careful about though is to only set the source or properties in the config to a single top level property file:
https://github.com/dbanksdesign/style-dictionary-node/blob/master/config.js

[chazzmoney]

I think Elliot's original request was maybe more on a convenience or code clarity thing for JSON. I'm not sure this solution would support his intent.

@elliotdickison What do you think?

[chazzmoney]

Also, @elliotdickison do you have your code somewhere that does the file prefixing as you mentioned?

[didoo]

Personally, I would avoid adding complexity to the package, if this can be achieved in other ways (as @dbanksdesign suggests "we can do this today with node module property files."). The great thing of Style Dictionary is its simplicity, let's keep it simple whenever we can.
I can add this as another example in the "examples" folder, with a proper Readme and explanation, but not a feature of SD.
Also, direct experience, a couple of weeks ago I did a HUGE (trust me, huge) refactoring of our entire design tokens suite in Badoo, and all I had to do was just move JSON files around, renaming them, but the suite never broke because of this separation between JSON content/organisation in term of tokens "paths" and organisation of the JSONs in term of files/directories. Let's not mix the two, it's a great feature of SD!

[chazzmoney]

@elliotdickison What do you think of the commentary in the last comment here? Would like to plan how we should handle this - whether to implement an option to support this in code, add an example of how to do this right now, or close. Thanks!

[elliotdickison]

I agree with @didoo in that we should keep complexity down. I was proposing this as a core change in behavior, not an opt-in feature. This proposal still makes more sense for my usage of style-dictionary, but if it's not how others are using the project I would rather keep my custom implementation than merge it as an opt-in feature and increase the complexity for everyone.

[chazzmoney]

@elliotdickison I know we are pretty distant time wise from your creation of this concern, but I did write a PR for this functionality, #253 . Would love to know if this is useful to you or not and if you have concerns about its usage.

[chazzmoney]

One way or another, whether we should add this functionality or decide not to add it, we should do so as part of 3.0

[davidyeiser]

For what it's worth I would find this very useful and would support it for 3.0. Though I agree this would have the potential to be a very disruptive change for some depending on their project structure.

What if we added the file path as an additional array to the prop?

Using the original core/color.json example, something like:

{
  value: '#ff0000',
  original: { value: '#ff0000' },
  name: 'red',
  attributes: {
    category: 'color',
    type: 'alert',
    item: 'red',
    subitem: undefined,
    state: undefined
  },
  path: [ 'color', 'alert', 'red' ],
  filePath: [ 'core' ]
}

[dbanksdesign]

@davidyeiser I like that idea! It would require some heavy modifications in the build process, but it is do-able.

[chazzmoney]

One way or another, whether we should add this functionality or decide not to add it, we should do so as part of 3.0

3.1 😬