-
Notifications
You must be signed in to change notification settings - Fork 108
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
rules skipped due to selector errors: legend+* -> Cannot read property 'type' of undefined #103
Comments
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. |
Do you know what selector is throwing this error? Some selector in the form |
It seems to come from Bootstrap (I see this warnings in v5.1.3, Angular 13.3.6) |
Getting the same (I think) with Tailwind typography plugin:
The only selectors, that error, seem to be with |
@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:
But I expect no warning about the selector, like this:
Interestingly enough |
This only happen with [email protected]
I've had a closer look at this bug, and I now think it comes from an incompatibility between It seems the most obvious solution would be to replace 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)... |
nice, I dont know how to fix but is that part can replace by critters/packages/critters/src/dom.js Line 17 in a590c05
|
Yes
I did not look for a replacement. But since it is the core feature of
You can try. But I am pretty sure that the @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 #103 (comment) ? |
@developit god, please guide us, we are willing to follow you sir 🦻 |
@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 Would you be able to discuss this issue with Angular team ? and see whether we can find a way forward for this ? |
@PowerKiKi, thanks for bring this up. I am not sure what was the reason why Although Maybe @janicklas-ralph can pitch in? |
@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... |
@PowerKiKi, I did bring this up among other issues with Critters. However, this will require some further discussions with the Chrome team |
Just to provide additional test-cases, all occurrences of the lobotomized owl selector in an Angular code base result in this warning. Eg:
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);
} |
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. |
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. |
The proper long-term solution is discussed in GoogleChromeLabs#103 (comment) but until then we should at least not break existing install that upgrade their dependencies. Without this we would get errors similar to: ``` rules skipped due to selector errors ``` Fixes GoogleChromeLabs#103
The proper, long-term solution is discussed in GoogleChromeLabs#103 (comment) but until then we should at least not break existing install that upgrade their dependencies. Without this we would get errors similar to: ``` rules skipped due to selector errors ``` Fixes GoogleChromeLabs#103
Just want to add this comment here to note that I followed @PowerKiKi's comment in #117 about overriding Proof: What I added to {
...
"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"
}
...
} |
No me funciono :/ |
Any fix yet? I am having a similar problem 2 months later.
Is this the same sort of problem? Or is this for something else (and I should post this error in some other place)? |
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: #117 |
@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 ? |
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. |
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 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). |
Hi, is there any update on this? When is this going to be fixed? Thanks! |
Solved by #124, which was released as 0.0.17. @gravityctrl, could you please close this issue ? |
@aditbisa, can you please provide an example of the problematic selector? |
The warning text
My codes
|
Encountered the same issue with the latest Angular 17 with
|
I have a similar issue in Angular 17.1.0: ▲ [WARNING] 2 rules skipped due to selector errors: |
@Evg772 "architect": {
"build": {
"configurations": {
"production": {
"optimization": {
"scripts": true,
"styles": {
"minify": true,
"inlineCritical": false <-- This one to false
},
"fonts": true
},
}
}
}
} |
@janicklas-ralph, could you please close this issue as it was solved by yourself a few months ago via #124, which was released as 0.0.17 ? |
npm install --save @fortawesome/fontawesome-free ng add ngx-bootstrap npm remove bootstrap npm install bootstrap@~5.1 package.jsonに以下を追記 "overrides": { "autoprefixer": "10.4.5" }, 削除 package-lock.json 削除 node_modules npm install styles.scssにfontawesomeのimport追加 styles.scssに.btn-cta、.btn-default追加 ※ それぞれなぜそうしているのかについて 1. ngx-bootstrap10.2.0はbootstrap5.2.xに対応しておらず、bootstrap5.1.xを使う必要がある 2. bootstrap5.1.xの場合、autoprefixer警告が出る。これを暫定的に対応するには10.4.5を入れる必要がある 3. legend+* -> Cannot read properties of undefined (reading 'type') は現時点で解決不可能。放置 ※ その他詳細 未解決問題1 legend+* -> Cannot read properties of undefined (reading 'type') GoogleChromeLabs/critters#103 未解決問題2(実害がないため解決する気がないらしい) No captured browser, open http://localhost:9876/ karma-runner/karma#2372
"Unknown pseudo-class :dir" me too |
Same goes for
|
Bug description:
When building angular using the
production
configuration, the stepGenerating 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:
Versions
Critters: 0.0.16
Angular: 13.3.0 - 13.3.2
ng-bootstrap: 12.0.2
Bootstrap: 5.1.3
The text was updated successfully, but these errors were encountered: