google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.88k stars 816 forks source link

Update lit dependency to 3.1 #4678

Closed jcoyne closed 7 months ago

elalish commented 7 months ago

Planning to send a PR? Would also be nice to know what this unblocks, though I agree that staying up-to-date with dependencies is a good idea.

jcoyne commented 7 months ago

I'm not planning on sending a PR. I just wanted to track this issue. I'm most concerned with https://github.com/lit/lit/pull/2103/files#diff-9204efca403aca3ce114a362b26ecd70d56cc7f3e093f895c9cfc2bd2b4f4800 which allows lit to be used via importmap on a CDN.

elalish commented 7 months ago

Does that issue flow down to MV? I believe MV works via importmap on a CDN currently.

jcoyne commented 7 months ago

I might be a bit confused about this matter. On this page https://modelviewer.dev/examples/postprocessing/ I see an importmap is used for three, but not for lit. My assumption was that lit needs to be in the importmap as well since it is a dependency in https://github.com/google/model-viewer/blob/master/packages/model-viewer/package.json#L86C6-L86C9

elalish commented 7 months ago

No, we generally bundle all of our dependencies so that no import maps are needed. For the special case of the optional postprocessing feature, we split it into three: model-viewer-module, model-viewer-effects, and the standard three.js library which they both depend on. Lit is still bundled (it's tiny).

jcoyne commented 7 months ago

Oh, cool! Thanks for explaining it to me.