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

Update CSS grammar to allow /deep/ combinator as first element of a selector #43

Closed
ghost opened this issue Jul 20, 2017 · 11 comments
Closed

Comments

@ghost
Copy link

ghost commented Jul 20, 2017

Hello,

I saw that a few versions ago it was added support to /deep/ combinator, but for me is not working. I'm using version 4.7
Code:
/deep/ .btngroupstyle { background-color: black; border-color: #8cd1fa; border-style: solid; border-width: 1.5px; height: 35px; }
Error:
Parse error at line 44 column 1: 34: border-color: #8cd1fa; 35: border-radius: 5px; 36: border-style: solid; 37: border-width: 1.5px; 38: height: 35px; 39: margin-bottom: 10px; 40: text-align: left; 41: width: 70px; 42: } 43: 44: /deep/ .btngroupstyle { ^ 45: background-color: #ffffff; 46: border-color: #8cd1fa; 47: border-style: solid; 48: border-width: 1.5px; 49: height: 35px; 50: } 51: 52: .buttonload { 53: background-color: #0098f3; 54: color: #ffffff;

Regards.

@racodond
Copy link
Owner

Hi @HiperSoke,

Thanks for your feedback!
Is this code snippet part of an SCSS or CSS file?

Thank you

David

@ghost
Copy link
Author

ghost commented Jul 21, 2017

Hi racodond,

Thanks for your fast answer. That code snippet its part of a css file.

Regards.

@ghost
Copy link
Author

ghost commented Jul 21, 2017

I changed the /deep/ combinator to ::ng-deep as is the one that we should use for Angular 4 projects and seems that is isnt supported by the plugin, at least it can process the whole file so I can see other issues. I think that maybe you could add ::ng-deep as a deprecated combinator.

Check this url: https://angular.io/guide/component-styles#deprecated-deep--and-ng-deep

Regards.

@racodond
Copy link
Owner

I changed the /deep/ combinator to ::ng-deep as is the one that we should use for Angular 4 projects and seems that is isnt supported by the plugin

/deep/ is considered as a CSS combinator. Thus, in CSS it cannot be the first element of a selector. Hence, the parse error you get.

Unlike CSS, it is possible to have a CSS combinator as first element of a selector in SCSS. Hence the support of /deep/ in SCSS when it is the first element of a selector.

I'm not familiar with /deep/. Thus, it is really possible to have it as first element of a selector? If so, it'll require a change in the grammar.

::ng-deep is considered as a pseudo-element, that's why it is properly parsed.

I think that maybe you could add ::ng-deep as a deprecated combinator

OK. ::ng-deep is considered as a pseudo-element for now. Does it make sense to mark it as obsolete/deprecated in rule "Obsolete pseudo-elements and pseudo-classes should not be used". Or doesn't it make sense to consider it as a pseudo-element in an Anglular context? And it should really be considered as combinator in terms of wording?

@ghost
Copy link
Author

ghost commented Jul 21, 2017

I'm not an CSS expert, but I saw in a few examples that you can use /deep/ at the begining of an CSS file (checking the documentation of DevExtreme suit for Angular projects),
.
About the ::ng-deep I saw that other development teams are cosidering it as an pseudo-element. So for me looks fine to add it to that "Obsolete pseudo-elements and pseudo-classes should not be used" rule.

Regards.

@racodond racodond changed the title /deep/ parse error Update CSS grammar to allow /deep/ combinator as first element of a selector Jul 21, 2017
@racodond racodond added this to the 4.8 milestone Jul 21, 2017
@racodond
Copy link
Owner

I'm not an CSS expert, but I saw in a few examples that you can use /deep/ at the begining of an CSS file (checking the documentation of DevExtreme suit for Angular projects)

OK, I'll add support to this syntax.
.

About the ::ng-deep I saw that other development teams are cosidering it as an pseudo-element. So for me looks fine to add it to that "Obsolete pseudo-elements and pseudo-classes should not be used" rule.

OK. I created a dedicated issue: #45

@vaclav-dvorak
Copy link

just my two cents:

angular/angular#17557

from what i read /deep/ support will be removed from chrome as its semantically incorrect and ::ng-deep is angular specific replacement...

in this light im not sure the resolution you come to is correct :)

@racodond
Copy link
Owner

Hi @vaclav-dvorak,

I'm not sure to understand.
You agree with "OK. I created a dedicated issue: #45"?
But you do not agree with "OK, I'll add support to this syntax."?

Even if it is deprecated, "/deep/" syntax usage should not lead to a crash while analyzing a CSS file with SonarQube. The parser should be able to parse this syntax (even if it is not widely used and will be less and less).

David

@racodond
Copy link
Owner

Also created new issue: #47

@vaclav-dvorak
Copy link

Sorry for not being clear.

From what i read it feels to me that /deep/ should be in deprecated list and ::ng-deep shouldn't (as its angulars response for dropping of /deep/ support).
Even though I feel that your statement : "/deep/ is considered as a CSS combinator. Thus, in CSS it cannot be the first element of a selector." is correct i don't know if its strict truth. It makes sense for parser to be able to parse it for sure.

@racodond
Copy link
Owner

OK. Got it.

From what i read it feels to me that /deep/ should be in deprecated list and ::ng-deep shouldn't (as its angulars response for dropping of /deep/ support).

They plan to drop support of ::ng-deep as well. See https://angular.io/guide/component-styles#deprecated-deep--and-ng-deep

The shadow-piercing descendant combinator is deprecated and support is being removed from major browsers and tools. As such we plan to drop support in Angular (for all 3 of /deep/, >>> and ::ng-deep). Until then ::ng-deep should be preferred for a broader compatibility with the tools.

Even though I feel that your statement : "/deep/ is considered as a CSS combinator. Thus, in CSS it cannot be the first element of a selector." is correct i don't know if its strict truth. It makes sense for parser to be able to parse it for sure.

OK

@racodond racodond modified the milestones: 4.8, 4.9 Aug 9, 2017
@racodond racodond removed this from the 4.9 milestone Aug 21, 2017
@ghost ghost closed this as completed Aug 25, 2017
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

No branches or pull requests

2 participants