ember-source is required for babel-plugin-ember-template-compilation #306
Conversation
files/__addonLocation__/package.json
Outdated
| @@ -36,6 +36,9 @@ | |||
| "@embroider/addon-shim": "^1.8.7", | |||
| "decorator-transforms": "^2.0.0" | |||
| }, | |||
| "peerDependencies": { | |||
| "ember-source": ">= <%= latestEmberSource %>" | |||
There was a problem hiding this comment.
What's the rationale to put the version hardcoded into index.js? We have versions everywhere right in the template file (here), so why deviate from this pattern?
I would get it if the versions is determined in a smarter way than being hardcoded, but that's not the case. Seems more confusing to me to maintain tbh.
There was a problem hiding this comment.
I couldn't figure out how to get the version. Maybe i missed something? LatestVersion is async, and the locals object is sync
We have versions everywhere right in the template file (here), s
Where? 😅
There was a problem hiding this comment.
IMO I would do same what we do in ember-cli addon blueprint - make it second most recent LTS version
There was a problem hiding this comment.
Folks can change it if they want
There was a problem hiding this comment.
Where? 😅
Like everywhere in https://github.com/embroider-build/addon-blueprint/blob/main/files/__addonLocation__/package.json? 😅
We have all those versions specifiers hardcoded in that file, why not also ember-source?
IMO I would do same what we do in ember-cli addon blueprint - make it second most recent LTS version
I agree. Also it needs to match what we have in the Readme and whatever ends up in test-app/config/ember-try.js as the minimum Ember version.
There was a problem hiding this comment.
I do want to drop ember older than built in types tho
There was a problem hiding this comment.
Will happen in another pr to unblock built in types
There was a problem hiding this comment.
Unrelated to this PR tho because we just need a template compiler.
I don't think that's true, as you are introducing a peer dependency on ember-source here, but not on the version we supported until now, which is 4.12, but on >= 5.4. Which means, without violating the peer dependency, you are supposed to require Ember 5.4+ now. But then the addon setup would be very inconsistent, as ember-try and Readme still refer to 4.12.
There was a problem hiding this comment.
ah I see what you mean
There was a problem hiding this comment.
fixed 58d2907
NullVoxPopuli
deleted the
ember-source-is-required-for-babel-plugin-ember-template-compilation
branch
CI is currently failing on main, as does
pnpm buildin a real generated project (which is what CI does).