Intelligently determine if value should be a string

[esr360]

I'm not sure if you're familiar with node-sass-json-importer, but one of the caveats of that tool is that you must double quote strings in your JSON, which essentially means you cannot guarantee that a valid JSON file will not break the Sass compiler.

I much prefer the implementation here which treats everything as a string and has the user selectively use unquote as desired; I had tried to get the same behaviour implemented into node-sass-json-importer to no avail thus far (pmowrer/node-sass-json-importer#5 (comment)).

However, I have a current open pull request which attempts to intelligently determine if a value should be treated as a CSS value or a string: pmowrer/node-sass-json-importer#70

It's quite a simple and effective solution which is working great in my rather large project so far (with over 40 JSON files), which ultimately boils down to attempting to render the value with Sass and seeing if it works or not - if it doesn't, then it is treated as a string:

export function shouldBeStringified(value) {
  try {
    sass.renderSync({
      data: `$foo: ${value};`
    });

    return false;
  } catch(error) {
    return true
  }
}

...would something similar work with this project, and is it a safe solution?

[xzyfer]

Hey I was not aware of that project.

Honestly it's probably a way better alternative to this. This importer is really just for me to get some hands on experience with the importer API to think about some improvements.

The suggested approach seems rather inefficient. I think there probably a cleaner way, but as you say the simplicity of opting into quoting is quite nice.

I'll give it a quick hack some time this week to see what I come up with.

[esr360]

The suggested approach seems rather inefficient.

I'm 100% open to this possibility, but would you mind explaining why? So far, from my use of the suggested approach, I am able to have whatever JSON I want without ever having to use unquote in my Sass, and without any of my values ever being the wrong type (and I have around 50 JSON files with all sorts of values).

It works like this:

function parseValue(value) {
  if (shouldBeStringified(value)) {
    return `"${value}"`;
  } else {
    return value;
  }
}

Can you provide an example of some JSON input where it wouldn't output the desired value in Sass?

[xzyfer]

The inefficiency is executing Node Sass for each value in shouldBeStringified. What I was suggesting is I think there is why to achieve the same behaviour without this overhead.

[esr360]

@xzyfer Ah, I'm with you now. In that case then yes, I'm sure you are right.

[esr360]

Thinking about this solution some more, I'm not convinced there's a more elegant way which doesn't involve executing Node Sass for each value in shouldBeStringified. Every value has the potential need to be stringified, and the only way we can know for sure whether it should be stringified is whether it will break the Sass compiler, and the only way we can know whether it will break the Sass compiler is to pass it to the Sass compiler.

If you wanted a more elegant solution, you would probably need some logic to determine whether or not a value should be stringified without passing it to the Sass compiler. And in almost 3 years no such logic has been found to be reliable (pmowrer/node-sass-json-importer#5).

Perhaps I've made some incorrect assumptions, though.

[xzyfer]

And in almost 3 years no such logic has been found to be reliable

I disagree with conclusion.

the fact that it will break your js code is not problem of this library
(pmowrer/node-sass-json-importer#5 (comment))

IMHO the difficulty that's been faced in prior work is due different project goals.

It is the goal of this project for types to be compatible and sharable between JavaScript and Sass. As such we lean on JSON types for things for like numbers, lists, and maps rather than trying to mangle them into a Sass string. This completely side steps the complication of dealing with space or comma separated list vs. strings with commas or spaces.

[xzyfer]

I have a proof of concept in #4 I plan to release as v2.0. Give it a shot and see if you can break it.

[esr360]

Thanks, I will definitely give it a shot and report back.

[esr360]

Sorry for the delay testing this, I've been experimenting with css-in-js so might take me a while to get around to it.

[esr360]

Finally getting back around to requiring a decent Sass JSON importer so ended up back here. This project is looking good, though the issue raised here #4 (comment) is one I experience too. If I get chance I will try and help out with a PR.

[jonscottclark]

@xzyfer How can we buy you some coffees? Really :)

@esr360 I tried doing some PR work but there's some sorcery within this codebase that I can't understand (mostly to do with how to write new tests). Godspeed!