css-element-queries icon indicating copy to clipboard operation
css-element-queries copied to clipboard

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

Open bodia-uz opened this issue 7 years ago • 5 comments
trafficstars

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

bodia-uz avatar Mar 02 '18 13:03 bodia-uz

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

marcj avatar Nov 21 '19 00:11 marcj

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:

  • a library my-awesome-lib, which uses css-element-queries.
  • an application which uses a bundle of my-awesome-lib and another instance of css-element-queries library

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.

bodia-uz avatar Nov 21 '19 12:11 bodia-uz

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.

marcj avatar Nov 21 '19 15:11 marcj

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)) {
  // ..
}

bodia-uz avatar Nov 21 '19 16:11 bodia-uz

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. 👍

marcj avatar Nov 21 '19 18:11 marcj