marcj / css-element-queries

CSS Element-Queries aka Container Queries. High-speed element dimension/media queries in valid css.
http://marcj.github.io/css-element-queries/
MIT License
4.27k stars 487 forks source link

Allow to use multiple instances of ElementQueries across multiple packages (third-party nominal type fix) #217

Open bodia-uz opened 6 years ago

bodia-uz commented 6 years ago

When app, that uses library with bundled css-element-queries, has own css-element-queries dependency, css-element-queries from library bundle conflicts with css-element-queries from app

for example: https://github.com/marcj/css-element-queries/blob/1.0.2/src/ElementQueries.js#L170-L179

marcj commented 4 years ago

What does this mean more concretely? I don't understand the use-case and root issue.

bodia-uz commented 4 years ago

Next code mutates dom element with custom property: https://github.com/marcj/css-element-queries/blob/master/src/ElementQueries.js#L170-L179

So if there is two instances of css-element-queries they will both use element.elementQueriesSetupInformation and element.elementQueriesSensor. The problem is, that library work incorrect in this case.

Usecase example: given:

After application initialization will be two instances of css-element-queries, which will lead to incorrect behavior. Especially if versions of css-element-queries are different.

marcj commented 4 years ago

That problem has nothing to do with css-element-queries but with your building step. Nominal types is broken here since your build step includes css-element-queries twice, with is not compatible and never will. You need to use the same css-element-queries version as your dependency so npm dedupes them into one package. Alternative is that your build script (like webpack) removes duplicates and makes sure only one instance is ever loaded. If your my-awesome-lib is already bundled then there's no way to make this work when both use different versions. Either you break API when falling back to the css-element-queries version of my-awesome-lib or vice-versa, break API of the packages. NPM and your build script is responsible to make it compatible, not the library code itself.

bodia-uz commented 4 years ago

use case: I as a developer, just use my-awesome-lib as is, and don't dive into lib internals. I don't know what dependencies it has. Just use it. Then I decide that I need media queries for element, install css-element-queries and faced with bugs.

Are you sure it's building a step issue here?

And if my-awesome-lib is bunded js file, I even can't do some magic with webpack deduplicate.

As a variant, in a new version, assigning elementQueriesSetupInformation to element can be replaced with some map:

map.set(element, queriesSetupInformation);
...
if (map.has(element)) {
  // ..
}
marcj commented 4 years ago

I see, actually from UX point of view that's a valid point and we should fix that.

Map doesn't work here since it would lead to a gigantic memory leak. WeakMap is not supported in IE < 11. We could however generate a string ID for each ElementQuery instance and use that as prefix for the DOM properties. If you want to provide a PR please go ahead. 👍