Clean up some Glimmer deprecations #33
Conversation
| expect(this.$('option:eq(2)')).to.be.selected; | ||
| expect(this.$('option:eq(0)').is('[selected]')).to.be.falsy; | ||
| expect(this.$('option:eq(1)').is('[selected]')).to.be.truthy; | ||
| expect(this.$('option:eq(2)').is('[selected]')).to.be.truthy; |
There was a problem hiding this comment.
I suprised that this doesn't work reliably when it always seems to have in the past. This is pretty vanilla chai-jquery, and I'm curious what changed.
There was a problem hiding this comment.
I'm not sure. My guess was that Glimmer wasn't able to set the actual DOM property, but only the attribute.
There was a problem hiding this comment.
I was wrong. The problem is that _updateValueMultiple is getting triggered on insert, which causes selection to get written to, overriding the passed-in value.
|
Thanks for undertaking this work @jamesarosen! The only question I have is how this will affect users of Ember < 1.13. It looks like most of the new apis are only used inside the tests, and the only usage of a new api in application code that I can see is change to I'm not opposed to increasing the minimum Ember version, just want to be deliberate about understanding what it is. |
|
When I did similar work for ember-i18n, I did increase the minimum version and bumped the major version of the library. |
|
Might be worth checking out: https://github.com/rwjblue/ember-cli-version-checker |
Prefer `{{#each foos as |bar|}}` to `{{#each foo in foos}}`. This
eliminates some deprecation warnings following the ember upgrade.
In Ember 1.13, `Ember.View.views` no longer exists. It has been replaced by
the (equally private) `container.lookup('-view-registry:main')`. This extracts
that lookup to a test helper so the reliance on a private API is at least
isolated.
In Glimmer, that can cause performance problems. Instead, we use `didInsertElement` to schedule an `afterRender` callback that sets the `x-option`'s `select`.
`Ember.ObjectController` has been deprecated. Using it causes a warning about proxying properties.
Ember views automatically call `.change()` on DOM change events, so we can use that hook instead of adding a change handler in `didInsertElement` and tearing it down in `willDestroyElement`. This is now the only place we need to call `_updateValueSingle` or `_updateValueMultiple`. We don't need to do that when the content changes from upstream since (in Glimmer) the component will render the difference automatically. Calling those in an observer on `content.@each` was causing upstream writes to happen unnecessarily.
`Ember.computed.alias` treats `undefined` specially, causing some writes to fail.
jamesarosen
force-pushed
the
glimmer-deprecations
branch
from
b19a868 to
4712739
Compare
|
This JSBin shows that we can support _updateForm: Ember.on('didRender', function() {
if (form) { this.$().attr('form', this.get('form')); }
}); |
Glimmer currently doesn't support binding the `form` attribute because of how it detects settable attributes vs properties. See emberjs/ember.js#11221
jamesarosen
force-pushed
the
glimmer-deprecations
branch
from
4712739 to
b693a37
Compare
|
I've updated this PR to bring support for |
|
That's awesome! I'm good with this as long as we figure out backward compatibility. Any thoughts @cowboyd? |
|
pagination-pager has a compatibility table:
|
|
Hey @jamesarosen, thank you so much for this PR!! It's really appreicated |
{{#each foo in foos}}to{{#each foos as |foo|}}getComponentByIdhelper that usesapp.__container__.lookup('-view-registry:main')since Ember no longer exposesEmber.View.viewssetindidInsertElemenentbecause that can cause performance problems in Glimmer; instead, defer thesetto theafterRenderqueueEmber.ObjectControllerin testschangehandler thatEmber.Components come with instead of adding a handler indidInsertElementviathis.$.on('change')content.@eachthat was writing changes back upstream and sending actions. In Glimmer, the component will get rerendered automatically from upstream changes; it only needs to notify when the source of the change is a DOM event within its elementvalueandselectiontonull, notundefined; the latter has problems with properties that usealiasformsince Glimmer doesn't support that at the moment. See Using attributeBindings for component in 1.11+ doesn't bind the form attr emberjs/ember.js#11221