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

Stylus doesn't throw error at not existed property in json #2629

Open
mars-abd opened this issue Sep 2, 2021 · 7 comments
Open

Stylus doesn't throw error at not existed property in json #2629

mars-abd opened this issue Sep 2, 2021 · 7 comments

Comments

@mars-abd
Copy link

mars-abd commented Sep 2, 2021

For example
data.json:

{
    "color": "black"
}

styles.styl:

$data = json('data.json', { hash: true })

div
    color: $data.someProp

Command:
npx stylus styles.styl
will compile:

div {
    color: ;
}

without any errors

Json func documentation:
https://stylus-lang.com/docs/bifs.html#jsonpath-options

@iChenLei
Copy link
Member

iChenLei commented Sep 2, 2021

Thanks for report.

@mars-abd
Copy link
Author

mars-abd commented Sep 2, 2021

It seems I have found out why stylus do this behaviour

red = #000

div
    color red // if no variable 'red' then stylus will do nothing and we will get there native color red css value

But it is so unobviously

@mars-abd
Copy link
Author

mars-abd commented Sep 2, 2021

But in this case:

div
    color $red

Stylus could throw error that 'red' variable is undefined

We have hundreds stylus files in project, and after some refactoring of styles data in json files we can't find all errors

@iChenLei
Copy link
Member

iChenLei commented Sep 22, 2021

# e.g add new stylus cli options ?
stylus iChenLei.styl -o iChenLei.css --json-check

@groenroos What's your opinion ?

@groenroos
Copy link
Collaborator

groenroos commented Sep 22, 2021

I don't know if a JSON-specific option like that would have a wide enough audience - but I do think in the wider view, there should be an option on whether to throw errors when any variables (JSON or otherwise) or properties/indices within hash variables don't exist (as per the StackOverflow thread linked above). Some kind of --strict mode or so.

As @mars-abd pointed out above, it would be easy to throw errors when dollar-prefixed variables don't exist - but then this would create a gotcha for people who prefer non-prefixed variables, where they might reasonably expect --strict to function for them and it doesn't.

// would throw an error
div
    color $foo

// would not throw an error, but would render as-is
// even though "foo" isn't a valid color
div
    color foo

So, either;

  1. Non-prefixed variables never trigger undefined errors, and we accept the behaviour will be different here, compared to prefixed variables; or
  2. Stylus has (and continues to maintain) a comprehensive list of every possible valid CSS literal non-numerical value (colours, directions, flex options, etc), and throws an error in --strict whenever you use something that isn't one of them and also isn't a defined variable

To put it in context of this issue, for consistency, I would see the missing JSON property error also as part of a --strict mode, that didn't affect current default behaviour.

@groenroos
Copy link
Collaborator

Having said that, according to the issue description there is a weird inconsistency, where the undefined JSON hash property will be rendered as empty, while other undefined variables get rendered as literals;

$colors = json('data.json', { hash: true })

// all the following are undefined
div
    color red
    background $blue
    border-color $colors.gray

would according to the above be currently rendered as;

div {
    color: red;
    background: $blue;
    border-color: ;
}

which is an unexplained inconsistency. I would expect the $colors.gray to also be rendered as a literal - regardless of whether it throws an error, or whether we make a --strict mode.

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

No branches or pull requests

3 participants