-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add dependenciesMeta.*.injected
, fix peer deps
#49
Conversation
|
I believe CI is failing as we are building the If so, I don't think it's worth it trying to fix this here just to make CI pass, as #45 will refactor the whole setup. But then both PRs are required to make CI green, which means we need to merge one (this?) in a red state and bring the changes into the other to (hopefully!) make things green again. Ok? |
@@ -31,7 +31,8 @@ | |||
"peerDependencies": { | |||
"validated-changeset": "^1.3.4", | |||
"ember-changeset": "^4.1.2", | |||
"ember-headless-form": "^0.0.0" | |||
"ember-headless-form": "^0.0.0", | |||
"ember-source": "^4.8.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the min version 4.4 or 4.8?
The try config has 4.4. last i looked, but if 4.8 is min required, then we can remove the function polyfill 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch. Yeah, I still have 4.4, that's why the polyfill was added. Accidental copy-paste error, will fix!
Yup, the download-built-package action, if it's still used, needs to run pnpm i -f again
Yup! This approach, and turbo in general won't work until injected dependencies can work without pnpm i -f |
Latest PRs like #45 and #44 are failing CI. After debugging, it seems again related to embroider-build/embroider#1332, in that
dependencySatisfies
(inember-modifier
this time) is seeing a wrong package ofember-source
(that is unused intest-app
, thus going into that empty package case).While a previous attempt to fix this by using
dependenciesMeta.*.injected
before didn't succeed, it seems to do so now!Also fixing peer deps now, as with
injected
the workaround of not having ember-source as a devDependency seems to not be needed anymore.