-
-
Notifications
You must be signed in to change notification settings - Fork 118
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
Move @embroider/macros to peerDeps #1030
Conversation
Hmm I'm not sure about this one... In the readme for that package it explicitly says to include in Agree that apps which don't use mirage won't need it but it being in the dep tree shouldn't have any negative affects.
If that's the case then this is an issue with Are you using the mirage handler in your app? Are you using embroider or classic build? |
Did you noticed it in development or production build? If I recall correctly, Embroider macros work differently for development and production builds. |
No build at all, just installing ember-file-upload added a ton of lodash to deps in our pnpm lock. |
We're not using embroider, just classic build. |
Why is that a problem? |
Because they are deps and added to the bundle. If they were devDeps, it would be no problem. |
Not all code of installed dependencies for into the bundle. Actually only a very small subset does. If everything works as expected, Please check if it affects the production bundle or not. It's only a problem of unexpected code is pushed I to the production build. |
It's weird that all the lodash things went from being devDeps to deps when we installed ember-file-upload though. I consider that incorrect if they aren't going into the build. |
I am not sure if this actually helps or not, but we noticed a lot of
lodash
being included in our app after installing ember-file-upload and my hypothesis is @embroider/macros should be a peerDep since it is just used in the mirage stuff and that the lodash is coming from it, but I could be totally wrong.