Variables in detached rulesets not visited by visitor?

[miyaokamarina]

I have the following plugin foo.js

module.exports = {
    install({ tree: { Quoted }, visitors }, manager) {
        class Visitor {
            constructor() {
                this.native = new visitors.Visitor(this);

                this.isPreEvalVisitor = true;
                this.isReplacing = true;
            }

            run(root) {
                console.log(
                    root
                        .rules[2]
                        .rules[0]
                        .arguments[0]
                        .value
                        .ruleset
                        .rules[0]
                        .value
                        .value[0]
                        .value[0]
                        .type,
                ); // 'Variable'

                return this.native.visit(root);
            }

            visitVariable(node) {
                console.log('visited');

                return new Quoted(`'`, 'bar', true);
            }
        }

        manager.addVisitor(new Visitor());
    },
};

and the following Less code

@plugin './foo';

.foo(@rules: {}) {
    :root.foo & {
        @rules();
    }
}

.foo {
    .foo({
        --foo: @foo;
    });
}

When trying to compile this code using lessc and/or less-loader, Less fails with error SyntaxError: variable @foo is undefined.

Variables in other contexts (in non-detached rulesets, at top level, in named detached rulests) are visited as expected:

@plugin './foo';

@bar: @foo;

.foo {
    --foo: @foo;
    --bar: @bar;
    @baz();
}

@baz: {
    --baz: @foo;
};

// ↓↓↓↓

.foo {
  --foo: bar;
  --bar: bar;
  --baz: bar;
}

Maybe other node types also affected, but I didn't test it.

Less version: 3.0.2.

[JakobJingleheimer]

Also exists in 3.0.0, 3.0.1, and 3.0.4. Does not exist in 2.7.3. I had to revert back.

[matthew-dean]

The error is confirmed; however, the node definitely is visited. Investigating...

[matthew-dean]

It appears that not only is the variable visited pre-eval in your example, but it also appears to be replacing. I think somehow a copy of the variable is being created in the process of parsing or creating the DR or DR call, then is eval'd separately.

@jshado1 Were you using @plugin syntax with 2.7.3?

[JakobJingleheimer]

@matthew-dean I'm just compiling variable overrides into Semantic-Org/Semantic-UI-LESS; I don't know what they're using.

[matthew-dean]

@jshado1 Variable overrides.... 🤔 Using a plugin? Why not just set a new value? Are they not globally scoped?

[JakobJingleheimer]

Some are not globally scoped. Ex site.variables are global, but button.variables apply only to button.less. I don't know how they achieve it.

[matthew-dean]

Okay! I figured out that for some reason, MixinCall arguments are not visited. It seems that it's an array of.... objects? instead of nodes? I've fixed it, but would like to fix some of the permissive parsing of custom properties to try parsing a value first. Otherwise you'll get a Variable today, but a Quoted after it's released, and your visitVariable would stop functioning. I'm just now sure how to make the parser "recover" from a bad path. I'm looking to tweak this code that was merged. - See this comment.

So if someone can help me understand how the Less parser works and addresses that comment, I'll do a PR for this fix.

[matthew-dean]

@jshado1 They probably just wrap certain vars and mixins like:

& {
  @scoped-var: value;
}

[matthew-dean]

Please review the PR - #3228