search

embroider-build / ember-auto-import

Zero config import from npm packages
Other
360 stars 109 forks source link

only check devDependencies when checking requested range of an app package #629

Closed mansona closed 3 months ago

mansona commented 3 months ago

I noticed this when I was linking a package that had a peerDependency range wider than it's devDependencies (ember-showdown-prism)

i don't understand why we would ever want to check the requested range of a dev depdency of a package 🤔 but perhaps there is a nuance that I don't understand here and we need to encode that nuance into the question that is being asked

mansona commented 3 months ago

Turns out fixing a lint error is very tough on the mobile app 🤣 I am very proud of my achievement 💪

ef4 commented 3 months ago

I think this code applies to apps too though. Which need their devDependencies.

We also have hasNonDevDepDependency() in this same class. Code like assertAllowedDependency is sensitive to the difference between apps and addons. If you found a spot that isn't, that is where the bug should be fixed.

mansona commented 3 months ago

I started trying to write down some context for this change and then I figured it would be better to just fix it rather than writing it down 😂 the requestedRange function is only used in one place so it was quite simple to add the new arg to it 👍

I'm pretty sure this fixes my issue but I'm wondering if you could help me place a test for this (unless you think it's not necessary?) 🤔