Skip to content

fix(components): remove ember-source as peerDep#2899

Merged
aklkv merged 1 commit into
mainfrom
fix/drop-ember-source-peer-as-dep
May 23, 2025
Merged

fix(components): remove ember-source as peerDep#2899
aklkv merged 1 commit into
mainfrom
fix/drop-ember-source-peer-as-dep

Conversation

@aklkv

@aklkv aklkv commented May 23, 2025

Copy link
Copy Markdown
Collaborator

📌 Summary

This follows this set of issues ember-cli/ember-addon-blueprint#35 which gives comprehensive reasoning for why

From PR descriptions elsewhere:

  • ember-source: removed because the embroider / auto-import know what we intend - it's not bad to have if someone manages their dep graph correctly, which is easier with pnpm, but not everyone gets it right, and folks have a hard time tracking down errors
  • @glimmer/tracking removed because it's a real package, but one we don't want to use. This comes up in embroider/vite where the presence of real packages always takes precedence over virtual packages. This is actually problematic because it can break reactivity in subtle ways, even if a dep graph is correct - allowing duplicates of dependencies, which for the glimmer internals, we don't want.

🛠️ Detailed description

📸 Screenshots

🔗 External links

Jira ticket: HDS-XXX
Figma file: [if it applies]


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

@aklkv aklkv requested a review from a team as a code owner May 23, 2025 03:01
@aklkv aklkv self-assigned this May 23, 2025
@vercel

vercel Bot commented May 23, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview May 23, 2025 3:08am
hds-website ✅ Ready (Inspect) Visit Preview May 23, 2025 3:08am

@aklkv aklkv merged commit 9a57bdf into main May 23, 2025
15 checks passed
@aklkv aklkv deleted the fix/drop-ember-source-peer-as-dep branch May 23, 2025 08:14
Comment on lines -122 to -124
"peerDependencies": {
"ember-source": "^3.28.0 || ^4.0.0 || ^5.3.0"
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aklkv @didoo how do we signal compatibility expectations? (how will consumers know which versions of ember-source our addon supports). this declaration resulted in a pnpm/yarn warning at installation informing users if they were outside the supported range.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this particular dependencies it's probably not possible as it is problematic. Follow linked issue for more details. Maybe if you ask that question there. But we can/should specify support range in the docs. At the same time our support rage is too wide as it is I wouldn't worry too much about potential issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants