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

Quote map keys (Scss) #9

Closed
nlemoine opened this issue Oct 16, 2019 · 9 comments · Fixed by #14
Closed

Quote map keys (Scss) #9

nlemoine opened this issue Oct 16, 2019 · 9 comments · Fixed by #14

Comments

@nlemoine
Copy link
Contributor

nlemoine commented Oct 16, 2019

Hi,

Thanks for this great package that makes using tailwind config in Sass much easier!

I think map keys should be quoted.

For example:

$colors: (
  black: #000,
  white: #fff,
);

Should be:

$colors: (
  "black": #000,
  "white": #fff,
);

Unquoted keys may lead to various issues.

@dobromir-hristov
Copy link
Owner

Hey. Yeah I was wondering about this myself when I wrote the library. This will cause a breaking change though, so it needs to be dealt with carefully.

@nlemoine
Copy link
Contributor Author

Besides CSS color names, I don't see any breaking change.

See the example: https://www.sassmeister.com/gist/54f8daa979e3fe41fe94a6a6c0d4e5ea

@damienroche
Copy link

actually i can't use this plugin because without quotes, i have some build failing
Module build failed .... SassError: Duplicate key.
using quotes avoid thoses sass errors for sure...

@tylerwiegand
Copy link

Same for me. maybe something changed but i need to use the quotes...

@tylerwiegand
Copy link

For those looking for a "for now" solution, adding the quotes here will work

  _buildObjectEntry(key, value, indent, index = 0, metricIndex) {
    return indentWith(`'${this._objectEntryKeySanitizer(key)}': ${this._sanitizePropValue(value)},\n`, indent + 2);
  }

I understand it's apparently a breaking change, but uhh...can we just make it an option an call it good?

@vdiaz1130
Copy link

Same here. 1/2 is same key as 2/4 and so is .5. I manually added quotes and everything works as expected. Then I tried @tylerwiegand suggestion which worked as well.

SassError: Duplicate key.

334 │ 1/2: 50%,
│ ━━━ first key
... │
338 │ 2/4: 50%,
│ ^^^ second key

@mpalpha
Copy link

mpalpha commented Jun 17, 2020

"Duplicate key" error with keys containing %. Likely has issues with . etc as well.

mpalpha added a commit to mpalpha/tailwindcss-export-config that referenced this issue Jun 19, 2020
mpalpha added a commit to mpalpha/tailwindcss-export-config that referenced this issue Jun 19, 2020
@mpalpha
Copy link

mpalpha commented Jun 19, 2020

#13 should fix the issue and should not break existing functionality for alphanumeric key names.

broken scss output: 👎

$width: (
  ...
  11/12: 91.666667%,
  full: 100%,
  ...
);

fixed scss output: 👍

$width: (
  ...
  '11/12': 91.666667%,
  full: 100%,
  ...
);

scss usage example:

// @import 'tailwind-variables'
@import 'tailwind-variables.scss';

@mixin modifiers(
  $map,
  $attribute,
  $prefix: '-',
  $separator: '-',
  $base: 'base'
) {
  @each $key, $value in $map {
    // escape slash
    @if (type-of($key) == string and str-index($key, '/')) {
      $key: str-insert($key, '\\', str-index($key, '/'));
    }
    &#{if($key != $base, #{$prefix}#{$key}, '')} {
      @if type-of($value) == 'map' {
        @include modifiers($value, $attribute, $separator);
      } @else {
        #{$attribute}: $value;
      }
    }
  }
}

.test {
  @include modifiers($width, 'width');
}

feel free to use my fork until #13 is merged or the issue is resolved another way.

npm i https://github.com/mpalpha/tailwindcss-export-config.git --save-dev

it is also a best practice to add a script>prepare entry in package.json to build dist like so:

  "scripts": {
    ...
    "prepare": "npm run build",
    ...

mpalpha added a commit to mpalpha/tailwindcss-export-config that referenced this issue Jun 19, 2020
…h-quotes

wrap quotes around non-alphanumeric keys - resolves dobromir-hristov#9
@dobromir-hristov
Copy link
Owner

dobromir-hristov commented Jun 21, 2020

Hello all, sorry for the delay. New job, new responsibilities... this one slipped.

Please someone checkout the PR I just attached - #14 and tell me whether it fits your needs.

It is optional, but forces this on all keys. This will most probably be the default in next version.

Thanks to @mpalpha for his work and PR.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants