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

@custom-media directive fails #2639

Closed
josephfinlayson opened this issue Jul 17, 2015 · 14 comments · Fixed by #2783
Closed

@custom-media directive fails #2639

josephfinlayson opened this issue Jul 17, 2015 · 14 comments · Fixed by #2783

Comments

@josephfinlayson
Copy link

Input LESS

.flex-grid {
    @import  "../../../../../bower_components/flexboxgrid/src/css/flexboxgrid.css";
}

Output CSS

.flex-grid {
  @import "../../../../bower_components/flexboxgrid/src/css/flexboxgrid.css";
}

How do I namespace CSS imports? Casting this file to less doesn't work as it uses newer CSS features. e.g.: @custom-media --md-viewport only screen and (min-width: 62em);

@seven-phases-max
Copy link
Member

If you don't want CSS to be parsed use inline. Though for most of the namespaced import use-cases only less import option can be valid since the compiler must parse and understand imported content to actually apply the namespacing properly, thus Less has to support @custom-media natively to parse and understand it too.

You can make a feature-request to support @custom-media natively, though adding drafted features is usually a bit annoying since those tend to change several times before stabilized.

Aside of above notice that you're actually trying to use wrong css file. The [``/src/css/flexboxgrid.css](https://github.com/kristoferjoseph/flexboxgrid/blob/master/src/css/flexboxgrid.css) one with @custom-media` is the source code for the `Myth` preprocessor (this code won't work in any browser even if Less would pass it), the compiled standard CSS files you should use are in `/css` dir.

@josephfinlayson
Copy link
Author

I see, I thought it was already part of the spec. I guess this would be part of a wider discussion on when LESS should support newer CSS features - custom media is part of this draft spec.

@matthew-dean
Copy link
Member

IMO Less will support features when they are implemented in more than one major browser, which I believe is along the same guidelines the W3C uses from moving something from Candidate Recommendation to Recommended status.

However, this is a gray area, because technically, in CSS, you can write any @-rule you want, and if the browser doesn't support it, it simply ignores it. Less doesn't really currently lint properties to test if they're valid, so explicitly passing only certain at rules is a little inconsistent but probably pragmatically necessary. Ideally though, if Less was going to act as a linter, it would simply check to see that CSS statements were correctly constructed, and that Less statements were valid.

So, I would support simply allowing any @-rule that passes the CSS syntax rules for valid construction of an @-rule, just like we do with blocks and parameters / values (unless @seven-phases-max or @lukeapage is like "Do you know how complicated that will make our parser?").

@seven-phases-max
Copy link
Member

So, I would support simply allowing any @-rule that passes the CSS syntax rules for valid construction of an @-rule

This makes sense. Actually currently Less does pass any unknown at-rule followed by {} block. So @custom-media (and any other unknown directive ending with ;) is a more rare exception. I suppose it's not that difficult to modify at-rule parser function to support either {} or ;-ending directives, it's just a matter of inventing more tests and making sure that a more relaxed parsing won't hit performance too much (obviously because too relaxed @* parsing interferes with variables syntax).

@stevenvachon
Copy link

Another reason why Less should be a PostCSS plugin.

@matthew-dean
Copy link
Member

@stevenvachon Making Less a PostCSS plugin would not change anything except parsing. The rules would still need to be evaluated by Less.

@stevenvachon
Copy link

No time; I have other projects. Less shouldn't have plugins as we already have PostCSS.

I'm pretty sure that ~"" doesn't work without a property, but I'll test on Monday. If not, perhaps we need a literal() for now so that we can still have future-CSS code:

literal("@custom-media --md-viewport only screen and (min-width: 62em);");

@seven-phases-max
Copy link
Member

Perhaps we need a literal() for now so that we can still have future-CSS code.

There're already @import (inline), ~"" and after all e().

But note that the main problem of that particular example is not that Less can't parse it - but in inability of Less to "bubble" this statement out of the wrapping .flex-grid (like it would do with ordinal @media) - just because Less has no idea what this statement is about.
In other words, Less can't and should not handle every proposed CSS draft feature (by now it's probably hundreds of those), if the feature requires some special handling. And it stays absolutely the same story if it's implemented as a PostCSS plugin.

@matthew-dean
Copy link
Member

and it stays absolutely the same story if it's implemented as a PostCSS plugin.

Correct. This is what I was saying. PostCSS doesn't solve anything without a preprocessing plugin to evaluate it (or "transform it" as PostCSS calls it). Even the author of PostCSS specifies that all that PostCSS technically is just a parser which creates an AST.

That said, I'm "working on" a postcss-less plugin. I say it in quotes because it depends on having extra time.

@stevenvachon
Copy link

Once many CSS features are implemented in the browsers, you guys will have to implement them in Less. They're already available as plugins for PostCSS, so it'd be less work for you and less waiting for us in the future.

@import (inline) "file.css"; worked well, by the wall. Thanks.

@seven-phases-max
Copy link
Member

I think I'll merge this to #2770 (this one is earlier but to not bother with label reassigning).
In fact if #2770 is properly fixed, the example in the initial post would work as expected since unknown at-rules are marked for bubbling anyway (and this should be fine for most of unknown/custom at-rules)).

@seven-phases-max
Copy link
Member

Merged to #2770.

@seven-phases-max
Copy link
Member

Re-opening this since #2770 is fixed, but bubbling for non-block at-rules is not enabled (see #2783).

@stale
Copy link

stale bot commented Nov 14, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 14, 2017
@stale stale bot closed this as completed Nov 28, 2017
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.

5 participants
@stevenvachon @matthew-dean @josephfinlayson @seven-phases-max and others