embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
330 stars 137 forks source link

Skip checking optional peer deps during dependency-validation #1829

Open NullVoxPopuli opened 3 months ago

NullVoxPopuli commented 3 months ago

Draft because I'm still debugging

Most immediately, this affects ember-data 4.12.5 (not usable in embroider because they have optional peers (or maybe not-correctly-forwarded peers?).

I don't know if this is the solve, as I don't really know haw to reason about checking if an optional peer declaration is correct or not. Feels like it may depend on runtime -- like if module loading happens pull on the optional peer deps -- so maybe skipping optional peers here is the best we can do?

Open to suggestions

ef4 commented 3 months ago

My first intuition here is that if a package declares an optional peer dep, it's OK for the dep to be not-present, but it's not ok for it to be present and not-the-same as what your peer packages are using.

So if we error for missing optional peer deps today, that is a bug we can fix. But if we're erroring because an optional peer is present-and-wrong, it's not a bug in the checker.