Skip to content
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

duplicate lit-html - possibly needs peerDependency or reexport everything? #682

Closed
daKmoR opened this issue May 23, 2019 · 14 comments
Closed

Comments

@daKmoR
Copy link
Contributor

daKmoR commented May 23, 2019

Steps to Reproduce

yarn add lit-html@1.0.0
yarn add lit-element@2.1.0

Expected Results

one version of lit-html

Actual Results

$ yarn list --pattern lit-html
yarn list v1.13.0
├─ lit-element@2.1.0
│  └─ lit-html@1.1.0
└─ lit-html@1.0.0
Done in 1.10s.

Suggestion

I see 2 possible solutions...

  1. lit-element adds lit-html only as peer dependency
  2. Everything that lit-html has get's exported so we can have only one dependency on lit-element

Note: right now we add lit-html as a dependency as we use features from it which are not reexported from lit-element (like directives).

Versions

  • lit-element: v2.1.0
@Westbrook
Copy link
Contributor

Isn't the answer here to use yarn add lit-html@^1.0.0 so that you aren't pinning your project to v1.0.0? That's what you see here: https://github.com/Polymer/lit-element/blob/master/package.json#L63

@Hypercubed
Copy link

Yeah... I think this can be an issue in many setups. Another example is loading from unpkg if there is a version mismatch. lit-element exports html and svg.. so those versions are always going to match. We cannot say the same for directive`.

@daKmoR
Copy link
Contributor Author

daKmoR commented May 23, 2019

we want to pin the version of lit-element and probably also lit-html if we import something from there. We want to pin as for some of the things we need to do we have a pretty tight coupling with lit-element/lit-html and we had problems in the past with automatic updates breaking our code...

not ideal... we know...

still pinning a version should not break everything right?

imho if everything is exported from lit-element then we don't need to add lit-html to our dependencies and therefore we could safely pin only lit-element

@Hypercubed
Copy link

Hypercubed commented May 23, 2019

I believe pinning everything will work. Just need to point out that this is needed for all systems (npm, unpkg, pkgdrop*).

* pkgdrop - my project for installing ESM

@Hypercubed
Copy link

Hypercubed commented May 24, 2019

Ideally, everyone would have a config where they get only one version of lit-html. But I wonder how hard it would be to also fix this internally to lit-element. I suspect it has to do with templateResult instanceof TemplateResult when we have two versions of TemplateResult (one in lit-element and one in user code).

@justinfagnani
Copy link
Contributor

As noted, the problem here is pinning to a specific version of lit-html. If you do this, you increase the chance of duplication.

Now, a reasonable package manager would have just chosen lit-html@1.0.0 without duplication, but npm and yarn don't have backtracking solvers. So if you really do need to pin, I'd recommend pinning everything that depends on that dependency too. But then again, this is what a package-lock is for.

I did think that if you put the pinned dependency first in your dependency list that npm would install that first, and that will more likely meet the constraint of following transitive dependencies on the package. You seem to be doing that with the order of install commands, so I'm not sure why there's duplication with lit-element/lit-html dependency.

@daKmoR is there a reason why you need to pin to a specific version? It pretty much breaks semver if you do that as a matter of course, because every release becomes a breaking change to you.

@Hypercubed
Copy link

@justinfagnani The issue, I believe, is that lit-element has a loose dep on lit-html@^1.0.0. If the consuming app (or cdn, or tooling) doesn't match that version range (for example if the app depends on the fixed version lit-html@1.0.0) then the versions can get out of sync. In this case it's that lit-html@1.1.0 matches lit-element but not the consuming apps.

I'm guessing either:

A) Users importing lit-element need to always match the lit-html dep range inside lit-element's package.json. This would also apply to any libraries that use lit-element.
B) Lit-element pins it's lit-html version and user and libs must match.
C) somehow get a looser coupling between lit-element and lit-html such that lit-element can consume a range of lit-html versions (as the lit-element's package.json implies).

@Hypercubed
Copy link

I guess one more option (as @daKmoR methoned) is moving lit-html to a peerDependency. Thinking more about it... isn't that what peer dependencys are for?

@justinfagnani
Copy link
Contributor

lit-html is a true dependency of lit-element

@Hypercubed
Copy link

Hypercubed commented May 25, 2019

I don't know a formal definition of a peer... my feeling is that a peer should be used when you publish a lib that uses a dependency that is also used by the consuming project. But that's just my take.

As for now the only thing we, consumers of lit-element, can do is ensure we always use the same lit-html version range as the latest version of lit-element.

@justinfagnani
Copy link
Contributor

That definition would include tons of common dependencies like lodash, rxjs. Peer dependencies create a burden for transitive dependents, and if one package truly depends on another it should just do so.

You don't have to use exactly the same version range as lit-element, but you should use the widest range that you can: the lowest lower bound and highest upper bound. The means for lit-html you should be at lit-html@^1.0.0.

@Hypercubed
Copy link

Agreed, maybe too broad. But, in-fact, rxjs is a peer of angular.

@magic-akari
Copy link

Maybe related issues: #589 #712 #724

@justinfagnani
Copy link
Contributor

We're going to keep lit-html as a regular dependency (changing that would be a breaking change) and continue to try to solve these problems in tooling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants