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

Guarded namespace unavailable when calling mixin with ruleset #3407

Open
ahultgren opened this issue Jul 16, 2019 · 6 comments · May be fixed by #3725
Open

Guarded namespace unavailable when calling mixin with ruleset #3407

ahultgren opened this issue Jul 16, 2019 · 6 comments · May be fixed by #3725

Comments

@ahultgren
Copy link

I keep running into issues today it seems.

Example:

#media {
  #desktop (@ruleset) {
    @media (min-width: 700px) {
      @ruleset();
    }
  }

  #mobile (@ruleset) {
    @media (max-width: 699px) {
      @ruleset();
    }
  }
}


@switch: demo;

.example {
  #lookup when (@switch = demo) {
    @not-found: 1;
  }

  @found: 2;
  
  #lookup2 {
    @also-found: 3;
  }

  
  #media#desktop({
    // Comment the next line and it will all compile
    error: #lookup[@not-found];
    success1: @found;
    success2: #lookup2[@also-found];
  });
}

Expected output:

@media (min-width: 700px) {
  .example {
    error: 1;
    success1: 2;
    success2: 3;
  }
}

Actual output: variable @not-found not found .

I managed to narrow it down to usage of the guard. Remove it and it works. Put the namespace within the ruleset and it works.

@matthew-dean
Copy link
Member

Again, this is an unsupported feature and there's nothing in the docs that would suggest this would work. If you're using a ruleset as data (as a map), then you can't use guards and expect to access it, since there's no lookup format for accessing that data.

Closing as not supported.

@matthew-dean
Copy link
Member

You can however use conditional logic in the value itself, as in:

@switch: demo;
#lookup {
  @not-found: if(@switch = demo, 1, 0);
}
.example {
  @media (min-width: 700px) {
    not-error: #lookup[@not-found];
  }
}

@ahultgren
Copy link
Author

If you're using a ruleset as data (as a map), then you can't use guards and expect to access it

I thought I had understood (though not agreed). Then I stumbled upon this:

#desktop (@ruleset) {
  @media (min-width: 700px) {
    @ruleset();
  }
}

@switch: demo;

.example {
  #lookup () when (@switch = demo) {
    @not-found: 1;
  }

  #desktop({
    wut: #lookup[@not-found];
  });
}

Expected and actual output:

@media (min-width: 700px) {
  .example {
    wut: 1;
  }
}

The only difference being that the namespace is parametric and that I'm very confused now. I seem to have found a solution to my problem, but please explain why one is supported but the other is not.

@matthew-dean
Copy link
Member

@ahultgren Oh interesting.

Well unfortunately, you're in a narrow space where something may work, but also isn't supported so isn't guaranteed to work in the future. In other words, this is undefined behavior.

As to why it works, it probably has something to do with the fact that guards on regular selectors was added later, so it all depends where parsing is split. That is, simple selectors can be treated as mixins, but aren't internally captured as mixins. They have a different node type, so accessing them is also somewhat different. In fact, the ruleset structures of DRs, mixins, and selectors is internally so different (unfortunately), that the lookup logic had to be unique per structure. It's just because they were added at different times, and then sub-features like guards were also added later at different times to each.

@ahultgren
Copy link
Author

Thanks for the explanation! I'm sure it makes sense to say that the behavior is undefined when looking at it from the perspective of the compiler. As an everyday user on the other hand, it seems weird that two features that each is supported can't be used together. It seems much more like all use cases weren't considered when adding one of the features (and is thus a bug) rather than explicitly not a supported use case.

Anyway this is not criticism, you're doing a great job and I'm not questioning this decision. I just wanted to share the perspective when you don't know the internals of less.

@matthew-dean
Copy link
Member

@ahultgren You're probably not wrong that the behaviors should at least align in some way, so I reopened this.

@lumburr lumburr linked a pull request May 7, 2022 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants