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

Questions about scss/map-deep #358

Open
7studio opened this issue Nov 22, 2019 · 6 comments
Open

Questions about scss/map-deep #358

7studio opened this issue Nov 22, 2019 · 6 comments

Comments

@7studio
Copy link
Contributor

7studio commented Nov 22, 2019

Hi,

I would like to understand the scss/map-deep behaviour because it seems very strange to me.

We will take the JSON file below as a basis for the reflection:

{
  "size": {
    "icon": {
      "small": { "value": "20px" },
      "medium": { "value": "24px" },
      "large": { "value": "32px" }
    }
  }
}
  1. The scss/variables will output SCSS variable list without !default flag.
$size-icon-small: 20px;
$size-icon-medium: 24px;
$size-icon-large: 32px;
  1. The scss/map-flat will output one single depth SCSS map.
$my-tokens: (
  'size-icon-small': 20px,
  'size-icon-medium': 24px,
  'size-icon-large': 32px
);
  1. The scss/map-deep will output a list of SCSS variables with !default flag and a SCSS map which uses the previously defined variables.
$size-icon-small: 20px !default;
$size-icon-medium: 24px !default;
$size-icon-large: 32px !default;

$my-tokens: (
  'size' : (
    'icon': (
      'small': $size-icon-small,
      'medium': $size-icon-medium,
      'large': $size-icon-large
    )
  )
);

I don't understand why the scss/map-deep is so different from the two first methods (scss/variables and scss/map-flat) and especially why values are not into the map like for the other map formatter.

The !default flag is very dangerous in this case because if the tokens file is not the first to be loaded, the values can be overwritten.

IMHO, the scss/map-deep formatter should not use a list of variables. If we want this list, we should take another way to generate it. Maybe something like below:

{
  "source": ["properties/**/*.json"],
  "platforms": {
    "scss": {
      "transformGroup": "scss",
      "files": [{
        "destination": "variables.scss",
        "format": "scss/variable",
      },
     {
        "destination": "map.scss",
        "format": "scss/map-deep",
        "mapName": "my-tokens"
      }]
    }
  }
}

For the moment, I'm using a custom format/formatter to remove these differences but maybe I'm wrong.

Can someone help me to understand these choices please?

Thank you ☺️

@dbanksdesign
Copy link
Member

TBH I'm not as competent at Sass as I used to be so I do not have a good answer. @didoo I believe you wrote these formats initially, do you have any thoughts on this?

@didoo
Copy link
Contributor

didoo commented Dec 4, 2019

@7studio look at this thread: #137

/cc @dbanksdesign

@chazzmoney
Copy link
Collaborator

@7studio This is still an issue, correct?

@7studio
Copy link
Contributor Author

7studio commented Jun 17, 2021

@chazzmoney IMHO, yes it is and much more since the themeable flag was introduced. To be less strict with my first point of view, the scss/map-deep formatter can use a list of variables but without the !default except if the themable flag is defined.

@chazzmoney
Copy link
Collaborator

@dbanksdesign lets clean this up too

@notlee
Copy link
Contributor

notlee commented Oct 19, 2021

The scss/map-deep will output a list of SCSS variables with !default flag and a SCSS map which uses the previously defined variables.

Hey, I just opened a PR to add support for the outputReferences option to scss/map-deep and noticed some of these differences between the Sass formatters. It would be much easier to add the outputReferences option to scss/map-flat too if it did reference previously defined variables in the same way as scss/map-deep

#720

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

5 participants