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

:host + ::ng-deep + @font-face produces wrong css #41751

Closed
artaommahe opened this issue Apr 21, 2021 · 12 comments
Closed

:host + ::ng-deep + @font-face produces wrong css #41751

artaommahe opened this issue Apr 21, 2021 · 12 comments
Assignees
Labels
area: core Issues related to the framework runtime core: CSS encapsulation P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Milestone

Comments

@artaommahe
Copy link
Contributor

artaommahe commented Apr 21, 2021

🐞 bug report

Affected Package

The issue is caused by package ¯\_(ツ)_/¯

Is this a regression?

¯\_(ツ)_/¯

Description

Using this

:host ::ng-deep {
  @font-face {
    // ...
  }
}

produces wrong css

@font-face {
  -shadowcsshost-no-combinator ::ng-deep {
    // ...
  }
}

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-host-ngdeep-fontface?file=src%2Fapp%2Fapp.component.scss

🔥 Exception or Error

image

🌍 Your Environment

Angular Version: 11.0.8

Anything else relevant?

We use such approach to scope global katex css to component's level (and so keep this styles with lazy loaded component, not in app's global styles.scss), that renders katex equations

:host ::ng-deep {
  @import '../../../../../../node_modules/katex/dist/katex';
}

where node_modules/katex/dist/katex contains selectors and @font-face definitions. Selectors are correctly parsed, font-faces nope and this breaks equations correct rendering (expected katex fonts are not loaded).
Similarly we use this approach for some other libs with same purpose - to scope global styles inside component

@alan-agius4 alan-agius4 transferred this issue from angular/angular Apr 21, 2021
@alan-agius4 alan-agius4 transferred this issue from angular/angular-cli Apr 21, 2021
@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Apr 21, 2021
@ngbot ngbot bot added this to the needsTriage milestone Apr 21, 2021
@spock123
Copy link

Has :ng-deep not been deprecated?

@Airblader
Copy link
Contributor

Airblader commented Apr 22, 2021

Has :ng-deep not been deprecated?

It has been deprecated ages ago, yes, but there is no replacement and it won't be removed until there is. From personal experience, in any decent sized project there is no way around using :ng-deep occassionally.

@artaommahe
Copy link
Contributor Author

Has :ng-deep not been deprecated?

there are a lot of cases when you have to change some styles for the stuff that you are not in control of (other libs e.x.). Usually it's better to do at the component level to not to pollute the app's global styles

@petebacondarwin
Copy link
Member

We should remove the SASS from this example to demonstrate the problem with pure CSS:

@font-face {
  :host ::ng-deep {
    font-family: "smth";
    font-weight: normal;
    font-style: normal;
  }
}

@petebacondarwin
Copy link
Member

I'll take a look at the bit in the compiler that parses these styles.

@petebacondarwin petebacondarwin added core: CSS encapsulation P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed area: core Issues related to the framework runtime labels Apr 23, 2021
@petebacondarwin petebacondarwin self-assigned this Apr 23, 2021
@ngbot ngbot bot modified the milestone: needsTriage Apr 23, 2021
@petebacondarwin petebacondarwin added the area: core Issues related to the framework runtime label Apr 23, 2021
@ngbot ngbot bot added this to the Backlog milestone Apr 23, 2021
@petebacondarwin
Copy link
Member

So I note that the required SCSS

:host ::ng-deep {
  @font-face {}
}

transpiles to this CSS

@font-face {
  :host ::ng-deep {}
}

but the @font-face specification does not allow for selectors within a @font-face rule: https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face#formal_syntax

So I don't believe that this CSS rule would make sense even if it was supported...

Can you show the desired output from the compiler working with vanilla CSS?

@artaommahe
Copy link
Contributor Author

artaommahe commented Apr 24, 2021

Just this

@font-face {
   font-family: "smth";
   font-weight: normal;
   font-style: normal;
}

@petebacondarwin
Copy link
Member

So basically what you are saying is that you have a mix of rules in an import, which happens to include a @font-face rule:

lib.scss

.marker { ... }
@font-face { ... }

And then you want to import that file:

:host ::ng-deep {
  @import 'lib';
}

so that the "normal" rules go deep and the font-face is just included:

:host ::ng-deep .marker { ... }
@font-face { ... }

Is that right? If so, then our CSS processing would do the right thing. I.e. it leaves @font-face alone.

If that is the case, then one could argue that it is SCSS that is doing the wrong thing here. It ought not to be injecting the :host ::ng-deep into the @font-face block.

@petebacondarwin
Copy link
Member

That being said, given that ::ng-deep is a bespoke Angular thing, perhaps we should help out here, and just strip all :host and ::ng-deep selectors from inside @font-face...

@artaommahe
Copy link
Contributor Author

Is that right? If so, then our CSS processing would do the right thing. I.e. it leaves @font-face alone.

Yep. As i mentioned in the first post, some libs have both regular selectors and font-faces in their styles, like this one https://cdn.jsdelivr.net/npm/katex@0.13.2/dist/katex.min.css (we're using it from npm package, same content there). So using it like this (to scope such global styles and have them with lazy loaded component)

:host ::ng-deep {
  @import '../../../../../../node_modules/katex/dist/katex';
}

produces wrong font-faces in the final css

@petebacondarwin
Copy link
Member

OK, I'll see if we can just strip out the :host and ::ng-deep markers in the case of font-face...
I wonder about the other "at-rule" types though??

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Apr 26, 2021
`@font-face` rules cannot contain nested selectors. Nor can they be
nested under a selector. Normally this would be a syntax error by the
author of the styles. But in some rare cases, such as importing styles
from a library, and applying `:host ::ng-deep` to the imported styles,
we can end up with broken css if the imported styles happen to contain
`@font-face` rules.

This commit works around this problem by sanitizing such cases (erasing
any scoping selectors) during emulated ShadowDOM encapsulation style
processing.

Fixes angular#41751
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Apr 26, 2021
`@font-face` rules cannot contain nested selectors. Nor can they be
nested under a selector. Normally this would be a syntax error by the
author of the styles. But in some rare cases, such as importing styles
from a library, and applying `:host ::ng-deep` to the imported styles,
we can end up with broken css if the imported styles happen to contain
`@font-face` rules.

This commit works around this problem by sanitizing such cases (erasing
any scoping selectors) during emulated ShadowDOM encapsulation style
processing.

Fixes angular#41751
jessicajaniuk pushed a commit that referenced this issue Apr 27, 2021
`@font-face` rules cannot contain nested selectors. Nor can they be
nested under a selector. Normally this would be a syntax error by the
author of the styles. But in some rare cases, such as importing styles
from a library, and applying `:host ::ng-deep` to the imported styles,
we can end up with broken css if the imported styles happen to contain
`@font-face` rules.

This commit works around this problem by sanitizing such cases (erasing
any scoping selectors) during emulated ShadowDOM encapsulation style
processing.

Fixes #41751

PR Close #41815
jessicajaniuk pushed a commit that referenced this issue Apr 27, 2021
`@font-face` rules cannot contain nested selectors. Nor can they be
nested under a selector. Normally this would be a syntax error by the
author of the styles. But in some rare cases, such as importing styles
from a library, and applying `:host ::ng-deep` to the imported styles,
we can end up with broken css if the imported styles happen to contain
`@font-face` rules.

This commit works around this problem by sanitizing such cases (erasing
any scoping selectors) during emulated ShadowDOM encapsulation style
processing.

Fixes #41751

PR Close #41815
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime core: CSS encapsulation P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants