Skip to content

Commit 506794e

Browse files
chriskrychoRobert Jackson
authored andcommitted
[BUGFIX LTS] do not throw on stable elementId
When refactoring from observers to setters during the 3.11 release (in 405d423), an apparently-transparent change caused a regression around invocations of components including `elementId=...`. *Any* rerender of the component which included `elementId` assignment now triggered the assertion, rather than *only* changes which actually updated the value of `elementId`, because all invocations strigger a `set` on the property. The simplest reproduction of this bug (given a component `foo-bar`): {{foo-bar elementId='stable' changingValue=this.fromBackingClass }} Changing the value `fromBackingClass` on the backing class *always* triggers the assertion, even though it is actually impossible for it to change the `elementId` value, because the assertion in the setter throws regardless of what the value is. (The observer-based did not throw in the same conditions because it would not retrigger when the observed key on the view did not change.) The fix is to check equality between the passed `elementId` value and the previously set value, and only throw if they differ. Fixes #18147 (cherry picked from commit da8c466)
1 parent 7ce6910 commit 506794e

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,39 @@ moduleFor(
109109
}
110110
}
111111

112+
['@test elementId is stable when other values change']() {
113+
let changingArg = 'arbitrary value';
114+
let parentInstance;
115+
this.registerComponent('foo-bar', {
116+
ComponentClass: Component.extend({
117+
init() {
118+
this._super(...arguments);
119+
parentInstance = this;
120+
},
121+
changingArg: changingArg,
122+
}),
123+
template: '{{quux-baz elementId="stable-id" changingArg=this.changingArg}}',
124+
});
125+
126+
this.registerComponent('quux-baz', {
127+
ComponentClass: Component.extend({}),
128+
template: '{{changingArg}}',
129+
});
130+
131+
this.render('{{foo-bar}}');
132+
this.assertComponentElement(this.firstChild.firstChild, {
133+
attrs: { id: 'stable-id' },
134+
content: 'arbitrary value',
135+
});
136+
137+
changingArg = 'a different value';
138+
runTask(() => set(parentInstance, 'changingArg', changingArg));
139+
this.assertComponentElement(this.firstChild.firstChild, {
140+
attrs: { id: 'stable-id' },
141+
content: changingArg,
142+
});
143+
}
144+
112145
['@test can specify template with `layoutName` property']() {
113146
let FooBarComponent = Component.extend({
114147
elementId: 'blahzorz',

packages/@ember/-internals/views/lib/views/states/in_dom.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ const inDOM = assign({}, hasElement, {
2222
get() {
2323
return elementId;
2424
},
25-
set() {
26-
throw new EmberError("Changing a view's elementId after creation is not allowed");
25+
set(value) {
26+
if (value !== elementId) {
27+
throw new EmberError("Changing a view's elementId after creation is not allowed");
28+
}
2729
},
2830
});
2931
}

0 commit comments

Comments
 (0)