Skip to content

Conversation

@jelhan
Copy link
Owner

jelhan commented May 10, 2025

Ember CLI addon blueprints add a peer dependency on ember-source: https://github.com/ember-cli/ember-addon-output/blob/c34e1239793a3c5faafbdffccf898d8bdf38ff73/package.json#L81-L83 Is there a RFC pending for implementation that this should be changed?

@jelhan jelhan added the enhancement New feature or request label May 10, 2025
@jelhan jelhan changed the title Remove unneeded peers Remove unneeded peer dependency on ember-source May 10, 2025
@NullVoxPopuli
Copy link
Author

No RFC needed, see: https://discord.com/channels/480462759797063690/568935504288940056/1370714948447113226

Changes to these sorts of things in the blueprints are to try to reduce problems for consumers, and are less so in need of opinions from the RFR process <3. I understand it is surprising tho. It was surprising to me when i found out we shouldn't be doing this as well!

  • 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.

Hope this helps!

@NullVoxPopuli
Copy link
Author

addon blueprints add a peer dependency on ember-source

I removed it from the v2 addon blueprint yesterday.
And removed @glimmer/tracking from the v2 app blueprint a couple days prior

@NullVoxPopuli
Copy link
Author

Opened a PR here for the v1

ember-cli/ember-cli#10697

@jelhan
Copy link
Owner

jelhan commented May 10, 2025

Thanks a lot for sharing all those details.

I tend to wait merging this one until the change got merged into the standard blueprints. I have only very limited time for maintaining this addon and others. Therefore I try staying as close as possible to the defaults to ease maintenance.

@NullVoxPopuli
Copy link
Author

NullVoxPopuli added a commit to emberjs/ember-qunit that referenced this pull request May 13, 2025
is extraneous, and the embroider/auto-import infra know where to get ember-source from.
It's not _wrong_ to include, but it requires folks' manage their deps correctly -- so we can be a little more forgiving to maintainers by just omitting this.


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.
 

Related:
- cibernox/ember-power-calendar#550
- lifeart/ember-click-outside-modifier#47
- tracked-tools/ember-async-data#854
- warp-drive-data/warp-drive#9986
- ember-animation/ember-animated#779
- tildeio/ember-element-helper#125
- CrowdStrike/ember-headless-form#574
- adopted-ember-addons/ember-sortable#620
- ember-cli/ember-app-blueprint#7
- ember-cli/ember-cli#10697
- ember-cli/ember-addon-blueprint#35
- embroider-build/addon-blueprint#339
- ember-polyfills/ember-functions-as-helper-polyfill#151
- jelhan/ember-style-modifier#312
- jmurphyau/ember-truth-helpers#211
- ember-modifier/ember-modifier#949
- tracked-tools/tracked-toolbox#211
- emberjs/ember-test-helpers#1543
- NullVoxPopuli/ember-resources#1189
- NullVoxPopuli/ember-modify-based-class-resource#20
- universal-ember/kolay#187
- universal-ember/reactiveweb#139
- universal-ember/ember-primitives#471
- universal-ember/docs-support#77
@aklkv aklkv mentioned this pull request Jul 22, 2025
@jelhan jelhan closed this in #323 Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants