Do not use global ember -- drop support for ember-source < 4.12 #973
Conversation
NullVoxPopuli
force-pushed
the
do-not-use-global-ember
branch
from
3a6ed80
to
1b552a2
Compare
addon/addon/template-cache.js
Outdated
| * | ||
| * Removed for RFC 1003 | ||
| */ | ||
| export let TEMPLATES = {}; |
There was a problem hiding this comment.
Is this a breaking change? If people could previously add to Ember.TEMPLATES?
There was a problem hiding this comment.
it's a private undocumented API, so if their code breaks, that's on them
There was a problem hiding this comment.
There was a problem hiding this comment.
another example: emberjs/ember.js#20632 (comment)
🙈
There was a problem hiding this comment.
It was definitely public once which is probably why I remember it.
I think it would be reasonable to make this a breaking change for ember-resolver, just so anybody that updates it before ember-source (& the deprecation) knows to look a bit closer. Doesn't really cost anything.
There was a problem hiding this comment.
It was definitely public once
uuggghh gross :(
(glad that era is over! haha)
I think it would be reasonable to make this a breaking change for ember-resolver,
fair enough! 🎉
There was a problem hiding this comment.
There's no point in keeping this. Just drop it.
Ember.TEMPLATES is the thing that gets auto-populated from your HTML like:
<script type="text/x-handlebars" data-template-name="my-component">hello world</script>A new major of ember-resolver could document that it no longer supports that pattern.
If people are manually stuffing templates into Ember.TEMPLATES, we don't want them to refactor that code to stuff the templates into this template-cache instead. Tell them to use register.
addon/addon/index.js
Outdated
|
|
||
| let getOwner; | ||
|
|
||
| if (macroCondition(dependencySatisfies('ember-source', '>= 4.11'))) { |
There was a problem hiding this comment.
Since this PR is now breaking anyway (which I agree with), how about not doing these macroCondition shenanigans and just supporting ember >= 4.11?
There was a problem hiding this comment.
done! I like this plan
addon/addon/template-cache.js
Outdated
| * | ||
| * Removed for RFC 1003 | ||
| */ | ||
| export let TEMPLATES = {}; |
There was a problem hiding this comment.
There's no point in keeping this. Just drop it.
Ember.TEMPLATES is the thing that gets auto-populated from your HTML like:
<script type="text/x-handlebars" data-template-name="my-component">hello world</script>A new major of ember-resolver could document that it no longer supports that pattern.
If people are manually stuffing templates into Ember.TEMPLATES, we don't want them to refactor that code to stuff the templates into this template-cache instead. Tell them to use register.
NullVoxPopuli
changed the title
|
CI is green, but ember-lts-4.4 is a required check, but this PR drops support for 4.4 |
|
@NullVoxPopuli ember-resolver/addon/package.json Line 29 in 5b5775c Is there any reason to ship still ember-resolver/addon/package.json Line 26 in 5b5775c |
Yup, deserves its own PR tho
Yeah, updating this just wasn't a focus from yesterday, but it probably should happen! |
|
Hi, @NullVoxPopuli and @ef4. I think you'll want to communicate the breaking change better in the release notes for When
Here's what to do to make CI pass again. |
|
I can copy notes for folks to copy-pasta if folks want to keep testing against older ember. All this to say, I updated the release notes: https://github.com/ember-cli/ember-resolver/releases/tag/v12.0.0 |
|
I would add that you can use ember-try to downgrade ember-resolver in the older scenarios. The same way ember-try is adjusting the ember-source version downward it can adjust ember-resolver downward. This actually improves the realism of the tests because you'd be testing under the conditions that apps on older ember-source versions are more likely to really be using. |
For emberjs/rfcs#1015
If folks come across this PR and are using
Ember.Templates, use register instead.