jungvonmatt / wekit

A Jamstack kit for Contentful, Hugo and Netlify.
https://www.wekit.dev
MIT License
14 stars 4 forks source link

container-query-polyfill: console error with class name ".container" #26

Open peterschewe opened 2 years ago

peterschewe commented 2 years ago

There is a strange bug with the container query polyfill.

This example...

.container {
  background: green;

  &--red {
    @include respond-below("md") {
      background: red;
    }
  }
}

...generates a console error

Uncaught (in promise) DOMException: Failed to execute 'querySelectorAll' on 'Document': '@media (max-width: 767px) ' is not a valid selector.
    at handleContainerProps (http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:161:29)
    at transpileStyleSheet (http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:126:7)
    at handleStyleTag (http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:461:20)
    at http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:475:55
    at NodeList.forEach (<anonymous>)
    at init3 (http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:475:38)
    at node_modules/container-query-polyfill/cqfill.js (http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:543:7)
    at __init (http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:4:56)
    at http://localhost:1313/js/main.1a2445c7b5155a02bed583ff05508b038bd28e8d8b0aa8fb72f4ed43b3fdb7e2.js:861:33

It seems to be a combination of the class name (if the string container is included) and the SCSS Parent Selector (&--).

Findings from the debugging:

This works...

.container {
  background: green;

  &.red {
    @include respond-below("md") {
      background: red;
    }
  }

  &__red {
    @include respond-below("md") {
      background: red;
    }
  }
}

...and also this

.container2 {
  background: green;

  &--red {
    @include respond-below("md") {
      background: red;
    }
  }
}
bezoerb commented 2 years ago

This has something to do with the class name .container followed by a - I think this line in the polyfill causing the error: https://github.com/GoogleChromeLabs/container-query-polyfill/blob/faf2600f7cd7b16e500f40676dd1b144005bd186/src/engine.ts#L291

Because of this error i needed to remove the .container styles in assets/scss/_base/base.scss in january. We should add a hint in the js where the polyfill is initialized

peterschewe commented 2 years ago

I had wondered a few times where the .container styles had gone 😅

psolbach commented 2 years ago

This definitely caused us some headaches and in general I believe we should not include progressive polyfills per default, only those ensuring x browser compatibility.

bezoerb commented 2 years ago

https://github.com/GoogleChromeLabs/container-query-polyfill#browser-support

bezoerb commented 2 years ago

I'm fine with not including this by default but we should keep it around somewhere so that it's easy accessible in case someone needs to use it.

bezoerb commented 2 years ago

There's already a PR in place to fix this issue but the repository is kind of stale as the team is trying to rewrite the polyfill to match the latest spec and align with the web platform tests.

https://github.com/GoogleChromeLabs/container-query-polyfill/pull/33