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

Non-standard at-rules #2770

Closed
corysimmons opened this issue Jan 10, 2016 · 42 comments · Fixed by #2783
Closed

Non-standard at-rules #2770

corysimmons opened this issue Jan 10, 2016 · 42 comments · Fixed by #2783

Comments

@corysimmons
Copy link

peterramsing/lost#197 (comment)

It seems like LESS parses non-standard at-rules. http://codepen.io/corysimmons/pen/OMmqwE?editors=010

http://www.w3.org/TR/css-syntax-3/#at-rule

Note: This specification places no limits on what an at-rule’s block may contain. Individual at-rules must define whether they accept a block, and if so, how to parse it (preferably using one of the parser algorithms or entry points defined in this specification).

According to spec, it would seem like LESS shouldn't try to parse at-rules.

@ai Are PostCSS users still recommended to use at-rules to create global settings?

@seven-phases-max
Copy link
Member

Yes, I'm afraid currently Less supports only this kind of unknown at-rules:

@unknown { ... }

@corysimmons
Copy link
Author

No way to escape to literal CSS in LESS? :^(

@seven-phases-max
Copy link
Member

@import (inline).

An in-place root (e.g. something out of either {} block) literal hack is also possible but has its problems of emitting a certain garbage:

.emit-at-root(@literal) {
    @namespace ~"whatever; @{literal}";
}

.emit-at-root("@lost foo bar");

@corysimmons
Copy link
Author

Thanks for your help but these solutions don't seem to work.

@import (inline) fails to import.

.emit-at-root includes @namespace.

@seven-phases-max
Copy link
Member

Not @namespace ~"@{literal}" but @namespace ~"whatever; @{literal}";. The hack is in ; inside the string.


As for @import (inline) realfile.css - it should be a real file with the CSS code you don't want to be parsed by Less.

@corysimmons
Copy link
Author

Okay thanks.

@ai
Copy link

ai commented Jan 10, 2016

Yeap, custom at-rules is still a best way to extend CSS syntax for PostCSS plugins.

@matthew-dean
Copy link
Member

According to spec, it would seem like LESS shouldn't try to parse at-rules.

Perhaps, but according to real-world usage, custom at-rules occur all the time by browser vendors. If you're using Less + PostCSS, the most compatible way would be to have Less at the top of your chain, I would think, and not send PostCSS settings into the Less parser.

@corysimmons
Copy link
Author

LESS is at the top of the chain. It's breaking a W3C valid rule.

@matthew-dean
Copy link
Member

@corysimmons Oh. You said the @-rule shouldn't be parsed. What you meant is that it should parse it, but it shouldn't try to evaluate it as a valid @-rule. There, you are a correct. This is a bug. I would be surprised if this hasn't already been marked as a bug somewhere already.

@seven-phases-max That hack is pretty fugly. 😬

@corysimmons
Copy link
Author

👍

@rjgotten
Copy link
Contributor

@matthew-dean
That hack is pretty fugly.

You think that's ugly? Try this on for size:

.emit-root-literal(@literal) {
  & {
    @plugin "literal.js";
    @ruleset: literal(~"@{literal}");
    @ruleset();
  }
}

That's a plugin function which emits a specially doctored detached ruleset which bubbles a specially doctored at-directive to the top of the tree, which has a specially doctored toCSS render implementation, which eventually emits the literal string that was originally passed into the function at the root level.

(...)

Yeah... I'm not particularly proud of that one, but it gets the job done when I need it.

@matthew-dean
Copy link
Member

@rjgotten Cripes, let's fix this.

@corysimmons
Copy link
Author

literal()?

It'd be nice if there was syntax to support multiline strings as well.

@stevenvachon
Copy link

Maybe emit warnings on such @ rules. For example, @extend copied from an SCSS file should be [manually] removed, but may have been missed.

@seven-phases-max
Copy link
Member

seven-phases-max commented Jan 11, 2016

@matthew-dean, @SomMeri I looked into the code and the fix should not be too hard though it's a bit tricky...
(I'm yet away from my Less development machine so can't do anything (well, tests), but here're few tips:
@unknown rules should get hasUnknown flag and then their hasBlock flag should be determined somewhere around of that hasUnknown parsing (e.g. if it ends with ; or {)).

@SomMeri
Copy link
Member

SomMeri commented Jan 12, 2016

Maybe we could allow arbitrary expression between an unknown at-rule and final ;.

@stevenvachon
Copy link

Any progress?

@corysimmons
Copy link
Author

👍 💯 🎉

@stevenvachon
Copy link

Please release

@plesiecki
Copy link

looking forward to release

@matthew-dean
Copy link
Member

@seven-phases-max Would you be willing to handle releasing for a new version?

@seven-phases-max
Copy link
Member

@matthew-dean I can't as I'm still away with the only old Mac, and here most of node and npm tasks are quite tedious.

@matthew-dean
Copy link
Member

@seven-phases-max Do we have documentation on the process?

@seven-phases-max
Copy link
Member

@matthew-dean It's just:

  • update version numbers and changelog (e.g. 33fa727)
  • grunt dist (the docs section is outdated)
  • make tag commit (e.g. 33fa727)
  • publish via npm (I'm not sure what about bower and gradle).

@SomMeri
Copy link
Member

SomMeri commented Jan 29, 2016

@matthew-dean @seven-phases-max Don't you need a key or password or something like that to publish to npm? In any case, I will have time Wednesday, but not before. If nobody releases till then and npm credentials are available (through some secure channel), I can do it then.

@seven-phases-max
Copy link
Member

@SomMeri

Don't you need a key or password or something like that to publish to npm?

Yes we do (I thought @matthew-dean has, if he does not we won't be able to publish of course).

@corysimmons
Copy link
Author

I thought people just had to be added as owners of a package? https://docs.npmjs.com/cli/owner

Is there some cool feature where they're required to type in their password each time they publish?

Btw, thanks for your work on this! 💯

@matthew-dean
Copy link
Member

I updated the version in git, but don't have npm access. Looks like the only 2 npm package owners are @cloudhead and @lukeapage, so one of them would either need to push to npm or add another owner.

@matthew-dean
Copy link
Member

@seven-phases-max Is there an easy / straightforward way to update the changelog? Or is pretty much a manual process of looking through commits?

@matthew-dean
Copy link
Member

Btw, I've sent a message to Luke and Alexis, so we should be able to make something happen soon. ^_^

@seven-phases-max
Copy link
Member

Is there an easy / straightforward way to update the changelog?

There's some git command that will list the descriptions of all commits after specified release/tag.
There's also this GitHub goody.

@matthew-dean
Copy link
Member

Thanks!

@lukeapage
Copy link
Member

Sorry for lack of action, I've given npm permission to @matthew-dean, let me know your npm username @seven-phases-max if you want it too

remember that the version needs changing in two places before grunt dist is run. That should really be documented..

@stevenvachon
Copy link

Why not just require("./package.json").version to avoid having to change the version in multiple places?

@matthew-dean
Copy link
Member

2.6.0 is released!

@lukeapage
Copy link
Member

@stevenvachon its been in the low priority list forever.. i guess the issue is the browser version.. either including the package.json in the browser build or a different solution. Probably the simple version would be okay..

@matthew-dean
Copy link
Member

@lukeapage Giving @seven-phases-max npm access is not a bad idea either, just so that releasing isn't a bottleneck. Do you think you could whip up a short doc of the steps for releasing? I'm pretty sure they were covered this time, but good to have around.

@seven-phases-max
Copy link
Member

@stevenvachon
Copy link

Thanks everyone. Very happy and excited with this release! No more *.css for my @custom-media.

@matthew-dean
Copy link
Member

And I learned a lot in the process! lol

@lukeapage
Copy link
Member

@seven-phases-max added

This issue was closed.
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.

10 participants