rules skipped due to selector errors: legend+* -> Cannot read property 'type' of undefined

gravityctrl commented 2 years ago

Bug description:

When building angular using the production configuration, the step Generating index html... produces following warning: 1 rules skipped due to selector errors: legend+* -> Cannot read property 'type' of undefined

See this screenshot:

Reproduction Steps

In Terminal using Angular CLI:

ng new testproject --style scss
cd testproject
ng add @ng-bootstrap/ng-bootstrap
npm run build

Versions

Critters: 0.0.16

Angular: 13.3.0 - 13.3.2

ng-bootstrap: 12.0.2

Bootstrap: 5.1.3

tnohelty commented 2 years ago

I am seeing the exact same error message and have the same versions as above. This was not happening until I just upgraded my project to the latest versions of these packages.

janicklas-ralph commented 2 years ago

Do you know what selector is throwing this error?

Some selector in the form legend+*. It would be helpful to test if you gave me the entire selector/rule

seimic commented 2 years ago

It seems to come from Bootstrap (I see this warnings in v5.1.3, Angular 13.3.6) bootstrap\scss_reboot.scss

Rarst commented 2 years ago

Getting the same (I think) with Tailwind typography plugin:

4 rules skipped due to selector errors:
  .prose :where(hr + *):not(:where([class~="not-prose"] *)) -> Cannot read properties of undefined (reading 'type')
  .prose :where(h2 + *):not(:where([class~="not-prose"] *)) -> Cannot read properties of undefined (reading 'type')
  .prose :where(h3 + *):not(:where([class~="not-prose"] *)) -> Cannot read properties of undefined (reading 'type')
  .prose :where(h4 + *):not(:where([class~="not-prose"] *)) -> Cannot read properties of undefined (reading 'type')

The only selectors, that error, seem to be with + * in them.

PowerKiKi commented 2 years ago

@janicklas-ralph you asked for a reproduction case. Here is the simplest I could do:

import Critters from 'critters';

const html = `
<!DOCTYPE html>
<style>
    .foo + * {
        color: red;
    }
</style>
`;

const critters = new Critters();
const inlined = await critters.process(html);
console.log(inlined);

It will output:

$ node index.mjs

1 rules skipped due to selector errors:
  .foo + * -> Cannot read properties of undefined (reading 'type')
Merging inline stylesheets into a single <style> tag skipped, no inline stylesheets to merge
Time 8.136544
<!DOCTYPE html><html><head>
</head><body></body></html>

But I expect no warning about the selector, like this:

$ node index.mjs

Merging inline stylesheets into a single <style> tag skipped, no inline stylesheets to merge
Time 8.136544
<!DOCTYPE html><html><head>
</head><body></body></html>

Interestingly enough <!DOCTYPE html> is required to trigger the bug. If absent, no warning will happen.

PowerKiKi commented 2 years ago

I've had a closer look at this bug, and I now think it comes from an incompatibility between parse5 which is used to parse HTML and css-select that is used to query CSS selectors. The document model is entirely different, and thus css-select try to access properties that never exist (node.prev).

It seems the most obvious solution would be to replace parse5 with htmlparser2, because it is part of the same ecosystem as css-select. However that also means that the DOM mutation must be ported to something else, most likely to domutils, and the same goes for the DOM serialization where the obvious choice would be dom-serializer.

All of that is a rather significant amount of work, and quite risky. It would probably be best if someone who know this codebase well would do it (instead of me)...

hiepxanh commented 2 years ago

nice, I dont know how to fix but is that part can replace by htmlparser2 ? or can we replace css-select with other? I try to upgrade css-select to 5.1.0 but it not work. can we create an issue for css-select author? @PowerKiKi https://github.com/GoogleChromeLabs/critters/blob/a590c05f9197b656d2aeaae9369df2483c26b072/packages/critters/src/dom.js#L17

PowerKiKi commented 2 years ago

is that part can replace by htmlparser2 ?

Yes

can we replace css-select with other?

I did not look for a replacement. But since it is the core feature of critters, I would try to keep using css-select to avoid behavior changes.

can we create an issue for css-select author?

You can try. But I am pretty sure that the css-select author will reject the issue. Because css-select works well with htmlparser2, and AFAIK it is not meant to work with anything else.

@developit, as the most recent committer, could you offer some guidance on how to proceed ? would you have the opportunity to fixes this yourself ? or would you accept a large PR that significantly rewrites the whole package as suggested in https://github.com/GoogleChromeLabs/critters/issues/103#issuecomment-1208236366 ?

hiepxanh commented 2 years ago

@developit god, please guide us, we are willing to follow you sir 🦻

PowerKiKi commented 2 years ago

@alan-agius4, since you opened #102 and that you are part of Angular team, I would like to bring your attention to this issue.

According to my findings there is a severe misconception where critters assumes css-select and parse5 to be compatible, but they are in fact totally unrelated, and cannot be used together.

I'd be inclined to say that it worked until now "by coincidence". But this is no longer the case with latest version of the package, which you actually get with a standard ng update. I suspect the warning is only one of many potential broken use case.

Would you be able to discuss this issue with Angular team ? and see whether we can find a way forward for this ?

alan-agius4 commented 2 years ago

@PowerKiKi, thanks for bring this up. I am not sure what was the reason why parse5 was used instead of htmlparser2.

Although css-select can support a custom adapter for parse5, see: https://github.com/fb55/css-select#options It does sound reasonable to me to use htmlparser2 directly as this would also avoid having to maintain a custom parse5 adapter like https://github.com/Polymer/css-select-parse5-adapter.

Maybe @janicklas-ralph can pitch in?

PowerKiKi commented 2 years ago

@alan-agius4 were you able to discuss this with Angular team and/or Critters team ?

I'd be willing to create a PR, but since this would be a large change I need to be sure there is a chance of merging it before doing anything...

alan-agius4 commented 2 years ago

@PowerKiKi, I did bring this up among other issues with Critters. However, this will require some further discussions with the Chrome team

johncrim commented 2 years ago

Just to provide additional test-cases, all occurrences of the lobotomized owl selector in an Angular code base result in this warning. Eg:

5 rules skipped due to selector errors:
  .Stack>*+* -> Cannot read properties of undefined (reading 'type')
  .Space--blocks>*+* -> Cannot read properties of undefined (reading 'type')
  .Row>*+* -> Cannot read properties of undefined (reading 'type')
  .Space--inline>*+* -> Cannot read properties of undefined (reading 'type')
  .Stack--dividers>*+* -> Cannot read properties of undefined (reading 'type')

I assume that means these CSS rules aren't inlined; so I could disable CSS inlining, but I can't use these selectors and also have them inlined.

Here's an example of the selector:

  .Row > * + * {
    margin-left: var(--inline-gap);
  }

PowerKiKi commented 2 years ago

I assume that means these CSS rules aren't inlined

This is correct. And because of that I think this issue should have a higher priority. At the very least Angular team should pin an exact version of package when it still worked.

developit commented 2 years ago

I believe the reason we went with parse5 over htmlparser2 was performance, but I'd have to dig up the PR. If htmlparser2 benchmarks similarly then I'd be fine switching to it, alternatively we could just patch the generated tree structure to include those accessor properties as getters on the prototype.

Jake-Thomas-Hall commented 1 year ago

Just want to add this comment here to note that I followed @PowerKiKi's comment in #117 about overriding css-select and it resolved my issues, I've got Angular 15.0.1 and Bootstrap 5.2.3.

Proof:

What I added to package.json:

{
   ...
   "overrides": {
       "css-select": "^4.2.1"
       // These may be optional, did not override these in my case as newer/same versions were already installed.
       "css-what": "^5.1.0"
       "domhandler": "^4.3.1"
   }
   ...
}

jimyhdolores commented 1 year ago

No me funciono :/

JacobHornbeck commented 1 year ago

Any fix yet? I am having a similar problem 2 months later. The error I am getting (it may not be the same problem, but I get a very similar error) is:

- Generating browser application bundles (phase: setup)...
√ Browser application bundle generation complete.
√ Browser application bundle generation complete.
- Copying assets...
√ Copying assets complete.
- Generating index html...
- Generating index html...
12 rules skipped due to selector errors:
  ion-content.custom-scroll-box::part(scroll) -> Pseudo-elements are not supported by css-select
  ion-menu::part(backdrop) -> Pseudo-elements are not supported by css-select
  :host-context([dir=rtl]) .ion-float-start -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-end -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-sm-start -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-sm-end -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-md-start -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-md-end -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-lg-start -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-lg-end -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-xl-start -> subselects_1.subselects[name] is not a function
  :host-context([dir=rtl]) .ion-float-xl-end -> subselects_1.subselects[name] is not a function
√ Index html generation complete.
error: unknown option '--project=*************************'

Is this the same sort of problem? Or is this for something else (and I should post this error in some other place)?

PowerKiKi commented 1 year ago

I didn't hear anything from Angular team or Google team for a while now. My simple PR that would at least temporarily fix the issue has received no official feedback yet: https://github.com/GoogleChromeLabs/critters/pull/117

PowerKiKi commented 1 year ago

@alan-agius4, it's been quite a while since we heard anything from from your side. Were you able to make things move forward somehow ?

alan-agius4 commented 1 year ago

We did raise this issue with the Chrome team, my understanding is that @janicklas-ralph will look into this and several other issues in Q1.

johncrim commented 1 year ago

In Angular and using yarn, I was able to make this warning go away using:

// package.json
  "resolutions": {
    "critters/css-select": "~4.2.1"
  },

Eg setting the css-select version to ~4.2.1 only within critters. I have other dependencies eg using css-select 5.1.0, so the selective dependency resolution feature is nice.

While this makes the warning go away (and the warning is there if I use newer versions of css-select like 4.3.0 or 5.1.0), our use does not prove that this provides a real fix, b/c the CSS that the warning complains about is not deemed critical (in our case).

arman-g commented 1 year ago

Hi, is there any update on this? When is this going to be fixed? Thanks!

PowerKiKi commented 1 year ago

Solved by #124, which was released as 0.0.17.

@gravityctrl, could you please close this issue ?

aditbisa commented 1 year ago

This problem appeared when I upgraded Angular v15 -> v16 (critters v0.0.16 -> v0.0.20)

alan-agius4 commented 1 year ago

@aditbisa, can you please provide an example of the problematic selector?

aditbisa commented 1 year ago

@aditbisa, can you please provide an example of the problematic selector?

The warning text

  :host-context([dir=rtl]) .ion-float-start -> Unknown pseudo-class :host-context([object Object])
  .ion-float-start:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-end -> Unknown pseudo-class :host-context([object Object])
  .ion-float-end:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-sm-start -> Unknown pseudo-class :host-context([object Object])
  .ion-float-sm-start:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-sm-end -> Unknown pseudo-class :host-context([object Object])
  .ion-float-sm-end:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-md-start -> Unknown pseudo-class :host-context([object Object])
  .ion-float-md-start:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-md-end -> Unknown pseudo-class :host-context([object Object])
  .ion-float-md-end:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-lg-start -> Unknown pseudo-class :host-context([object Object])
  .ion-float-lg-start:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-lg-end -> Unknown pseudo-class :host-context([object Object])
  .ion-float-lg-end:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-xl-start -> Unknown pseudo-class :host-context([object Object])
  .ion-float-xl-start:dir(rtl) -> Unknown pseudo-class :dir
  :host-context([dir=rtl]) .ion-float-xl-end -> Unknown pseudo-class :host-context([object Object])
  .ion-float-xl-end:dir(rtl) -> Unknown pseudo-class :dir
  body ion-button::part(native):is(button) -> Pseudo-elements are not supported by css-select
  ion-content::part(scroll) -> Pseudo-elements are not supported by css-select
  ion-toast::part(container) -> Pseudo-elements are not supported by css-select
  ion-item.app-item::part(native) -> Pseudo-elements are not supported by css-select
  ion-item.app-item.app-item-active::part(native) -> Pseudo-elements are not supported by css-select
  ion-select.app-select::part(icon) -> Pseudo-elements are not supported by css-select

My codes

/** global.scss */
...
@import './styles/app-styles.scss';
...

/** ./styles/app-styles.scss */
...
/* Select */
ion-select.app-select {
  --background: var(--app-input-background);
  border: 1px solid var(--app-input-border-color);
  border-radius: var(--app-border-radius);
  --padding-start: 8px !important;
  --padding-end: 8px;

  &::part(icon) {
    top: 50%;
  }
}
...

anisabboud commented 9 months ago

Encountered the same issue with the latest Angular 17 with ::part CSS selector (ng serve / ng build with ESBuild): "Pseudo-elements are not supported by css-select"

`.scss`

math-field::part(content) {
  padding: 6px;
}

`ng serve` / `ng build`

$ ng build
⠏ Building...
▲ [WARNING] 7 rules skipped due to selector errors:
  math-field::part(content) -> Pseudo-elements are not supported by css-select
  ...

See: https://github.com/fb55/css-select/blob/b1f29b83a5590e3fe34ebb5f6856fec6327b527b/src/general.ts#L26-L28

::part is a relatively new (~2020) pseudo-element shadow DOM selector, but it's supported by all major browsers: https://caniuse.com/mdn-css_selectors_part. Perhaps the css-select dependency is a bit outdated? The last release is ~2 years old https://github.com/fb55/css-select.

Evg772 commented 9 months ago

I have a similar issue in Angular 17.1.0: ng build

▲ [WARNING] 2 rules skipped due to selector errors: .form-floating>~label -> Did not expect successive traversals. .form-floating>~label -> Did not expect successive traversals.

seimic commented 9 months ago

@Evg772 Try to turn off the inlining of critical css in optimization section of the build profile? (Found here https://stackoverflow.com/questions/76798190/i-get-error-form-floatinglabel-in-ng-build and here https://github.com/ng-bootstrap/ng-bootstrap/issues/4306)

"architect": {
  "build": {
    "configurations": {
      "production": {
        "optimization": {
          "scripts": true,
          "styles": {
            "minify": true,
            "inlineCritical": false  <-- This one to false
          },
          "fonts": true
        },
      }
    }
  }
}

Moises56 commented 9 months ago

Esto Funciono Para mi, en Angular 17

PowerKiKi commented 8 months ago

@janicklas-ralph, could you please close this issue as it was solved by yourself a few months ago via https://github.com/GoogleChromeLabs/critters/pull/124, which was released as 0.0.17 ?

vsTianhao commented 6 months ago

"Unknown pseudo-class :dir" me too

nur-psoehnlein commented 3 months ago

Same goes for ::slotted…

▲ [WARNING] 21 rules skipped due to selector errors: .layout-grid ::slotted() -> Pseudo-elements are not supported by css-select .layout-list ::slotted() -> Pseudo-elements are not supported by css-select .layout-carousel ::slotted(*) -> Pseudo-elements are not supported by css-select […]

kara commented 1 week ago

Closing this issue because ownership of Critters has moved to the Nuxt team, who will be maintaining the project going forward. This repo is archived and won't receive any future updates.

If the issue is still relevant, please re-post it to the actively-maintained fork at https://github.com/danielroe/beasties.

GoogleChromeLabs / critters