Stylus: Sometimes the code generated by preprocessor doesn't match configured vars

Created on 10 Dec 2017  路  11Comments  路  Source: openstyles/stylus

  • Browser: Firefox 58.0b10
  • Operating System: Windows 7
  • Screenshot:


After rapidly dragging around on the colorpicker:
image
The style object in background page:
image

There are at least 2 problems:

  1. The style used by apply.js doesn't match the style in the background.
  2. Preprocessor uses an old config to compile the source / The config is changed after the source is compiled.
bug

All 11 comments

Need to debounce saving I guess.

Does the fix work?

On a related note, it'd be nice if simple variable values like colors - basically everything except text I guess - would be directly changed in the sections code without the need to reparse the entire style.

30fa967 does fix it. It just avoided rapid save() though. The main problem probably is that style is not deep copied in config dialog and is shared with background page.

if simple variable values like colors - basically everything except text I guess - would be directly changed in the sections code without the need to reparse the entire style.

I think it is possible for @preprocessor default but not stylus-lang and uso.

vars is deep-copied in config dialog so there's no problem AFAICT.
As for the simple replacement, maybe it's possible to detect if a given variable is simple and not used by other variables?

styleVars is not cloned.
https://github.com/openstyles/stylus/blob/30fa9671c8171d8e0e9bef34cb8fea54d50d1726/manage/config-dialog.js#L121

I made an experimental branch:
https://github.com/openstyles/stylus/compare/master...eight04:dev-partial-fix

Following is always true in popup:

    console.assert(bgStyle === style);
    console.assert(bgVars === style.usercssData.vars);

Maybe save() can be throttled instead of debounced.

I think it is a good idea to debounce save(). It's smoother when dragging on colorpicker and doesn't heat up my CPU. I only tested it on Firefox though.


The problem is you don't know where is the variable in the compiled code. For example:

/*
@var color my-color "My Color" red
*/
body {
    color: my-color;
    border: red;
}

Would be compiled to:

body {
    color: red;
    border: red;
}

It's a lot easier for @preprocessor default:

/*
@var color my-color "My Color" red
*/
body {
    color: var(--my-color);
    border: red;
}

Would be compiled to:

:root {
    --my-color: red;
}
body {
    color: var(--my-color);
    border: red;
}

Okay, so we just deepClone the style, right?
manage.js does that already by assigning entry.styleMeta = getStyleWithNoCode(style)

You might have to test if it works on Firefox. I originally used deepCopy on style.usercssData, but the object became a dead object after the popup is closed. So I changed it to BG.deepCopy:
https://github.com/eight04/stylus/blob/bca64587ad7ddf24d71122d6e40db8bd208d329d/manage/config-dialog.js#L126

I have no idea how does Firefox handle the object shared with background page.

Ah, yes, a second deep-copying is needed: BG.usercssHelper.save(BG.deepCopy(style))

So it's fixed, apparently.

I've solved the performance problem mentioned above differently: the new csslint-refactor branch reuses parsing results so tweaking GitHub Dark takes only ~30ms instead of ~300ms.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

whyseman picture whyseman  路  30Comments

schomery picture schomery  路  35Comments

xt0rted picture xt0rted  路  51Comments

tophf picture tophf  路  26Comments

eight04 picture eight04  路  30Comments