GoogleChromeLabs / critters

🦔 A Webpack plugin to inline your critical CSS and lazy-load the rest.
https://npm.im/critters-webpack-plugin
Apache License 2.0
3.42k stars 108 forks source link

Pin `css-select` to latest compatible version #117

Closed PowerKiKi closed 1 year ago

PowerKiKi commented 1 year ago

The proper long-term solution is discussed in https://github.com/GoogleChromeLabs/critters/issues/103#issuecomment-1208236366 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 #103

PowerKiKi commented 1 year ago

@developit, @alan-agius4, this is at least the temporary solution for the issue we talked about in #103. Until an important refactor is done. This should be a no-brainer to merge and release as a patch version. Could you please try to bring this up to the Google team/Angular team ?

It would solve the issue for a lot of people that otherwise have to resort to poor workaround as seen in

XhmikosR commented 1 year ago

I was about to suggest this too after digging into my node_modules and found that the error is coming from a minor version bump in css-select.

My only concern is that angular 13 and 14 that I use, are using critters@0.0.16:

C:\Users\xmr\Desktop\app\dev>npm ls critters
app@0.0.0 C:\Users\xmr\Desktop\app\dev
`-- @angular-devkit/build-angular@14.2.10
  `-- critters@0.0.16

So, in order for this to be fixed, we'll need a new 0.0.x version and angular to update to that patch version and release new patch versions of angular...

PowerKiKi commented 1 year ago

Indeed all supported Angular versions pinned the exact version to 0.0.16, so they will need to release a new Anglar version in order for this PR to have any effect:

That's not ideal, but hopefully @alan-agius4 will be able to help us with that when this PR is released...

In the mean time, if you use yarn, you can add this to your package.json:

{
    ...
    "resolutions": {
        "css-select": "4.2.1",
        "css-what": "5.1.0",
        "domhandler": "4.3.1"
    },
    "dependencies": {
        ...
    }
}
Jake-Thomas-Hall commented 1 year ago

Indeed all supported Angular versions pinned the exact version to 0.0.16, so they will need to release a new Anglar version in order for this PR to have any effect:

That's not ideal, but hopefully @alan-agius4 will be able to help us with that when this PR is released...

In the mean time, if you use yarn, you can add this to your package.json:

{
    ...
    "resolutions": {
        "css-select": "4.2.1",
        "css-what": "5.1.0",
        "domhandler": "4.3.1"
    },
    "dependencies": {
        ...
    }
}

Would just like to add, thank you for this workaround. For anyone using npm and who has gone down this rabbit hole, below is what you need to add to your package.json for now until this change to critters is merged and updated into Angular.

Note that I've got Angular 15.0.1, Typescript 4.8.4 and Bootstrap 5.2.3 installed.

Overriding the version of css-select from 4.2.0 to 4.2.1 fixed the warning for me:

image

package.json:

{
   ...
   "overrides": {
       "css-select": "^4.2.1"
       // These may be optional, check the versions present in your package-lock.json, if they're already this version or later no need to include
       "css-what": "^5.1.0"
       "domhandler": "^4.3.1"
   }
   ...
}
weilinzung commented 1 year ago

Indeed all supported Angular versions pinned the exact version to 0.0.16, so they will need to release a new Anglar version in order for this PR to have any effect:

That's not ideal, but hopefully @alan-agius4 will be able to help us with that when this PR is released... In the mean time, if you use yarn, you can add this to your package.json:

{
    ...
    "resolutions": {
        "css-select": "4.2.1",
        "css-what": "5.1.0",
        "domhandler": "4.3.1"
    },
    "dependencies": {
        ...
    }
}

Would just like to add, thank you for this workaround. For anyone using npm and who has gone down this rabbit hole, below is what you need to add to your package.json for now until this change to critters is merged and updated into Angular.

Note that I've got Angular 15.0.1, Typescript 4.8.4 and Bootstrap 5.2.3 installed.

Overriding the version of css-select from 4.2.0 to 4.2.1 fixed the warning for me:

image

package.json:

{
   ...
   "overrides": {
       "css-select": "^4.2.1"
       // These may be optional, check the versions present in your package-lock.json, if they're already this version or later no need to include
       "css-what": "^5.1.0"
       "domhandler": "^4.3.1"
   }
   ...
}

We have to do this in order to fix it temporarily, without nested override won't do anything.

  "overrides": {
    "@angular-devkit/build-angular": {
      "critters": {
        "css-select": "4.2.1"
      }
    }
  }
XhmikosR commented 1 year ago

@developit friendly ping 🙂

alan-agius4 commented 1 year ago

This has been addressed as part of 0.17.0

PowerKiKi commented 1 year ago

Superseeded by https://github.com/GoogleChromeLabs/critters/pull/124, which was released as 0.0.17