Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix an issue in the custom elements bundle where if you set a property on an element programatically before it was upgraded it would shadow the accessors and therefore break any watchers as well. See https://developers.google.com/web/fundamentals/web-components/best-practices\#lazy-properties #2949

Merged
merged 7 commits into from
Jul 7, 2021
2 changes: 1 addition & 1 deletion src/runtime/connected-callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const connectedCallback = (elm: d.HostElement) => {

// Lazy properties
// https://developers.google.com/web/fundamentals/web-components/best-practices#lazy-properties
if (BUILD.prop && BUILD.lazyLoad && !BUILD.hydrateServerSide && cmpMeta.$members$) {
if (BUILD.prop && !BUILD.hydrateServerSide && cmpMeta.$members$) {
Object.entries(cmpMeta.$members$).map(([memberName, [memberFlags]]) => {
if (memberFlags & MEMBER_FLAGS.Prop && elm.hasOwnProperty(memberName)) {
const value = (elm as any)[memberName];
Expand Down
39 changes: 39 additions & 0 deletions src/runtime/proxy-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,45 @@ export const proxyComponent = (Cstr: d.ComponentConstructor, cmpMeta: d.Componen
prototype.attributeChangedCallback = function (attrName: string, _oldValue: string, newValue: string) {
plt.jmp(() => {
const propName = attrNameToPropName.get(attrName);

// In a webcomponent lifecyle the attributeChangedCallback runs prior to connectedCallback
// in the case where an attribute was set inline.
// ```html
// <my-component some-attribute="some-value"></my-component>
// ```
//
// There is an edge case where a developer sets the attribute inline on a custom element and then programatically
// changes it before it has been upgraded as shown below:
//
// ```html
// <!-- this component has _not_ been upgraded yet -->
// <my-component id="test" some-attribute="some-value"></my-component>
// <script>
// // grab non-upgraded component
// el = document.querySelector("#test");
// el.someAttribute = "another-value";
// // upgrade component
// cutsomElements.define('my-component', MyComponent);
// </script>
// ```
// In this case if we do not unshadow here and use the value of the shadowing property, attributeChangedCallback
// will be called with `newValue = "some-value"` and will set the shadowed property (this.someAttribute = "another-value")
// to the value that was set inline i.e. "some-value" from above example. When
// the connectedCallback attempts to unshadow it will use "some-value" as the intial value rather than "another-value"
//
// The case where the attribute was NOT set inline but was not set programmatically shall be handled/unshadowed
// by connectedCallback as this attributeChangedCallback will not fire.
//
// https://developers.google.com/web/fundamentals/web-components/best-practices#lazy-properties
//
// TODO(STENCIL-16) we should think about whether or not we actually want to be reflecting the attributes to
// properties here given that this goes against best practices outlined here
// https://developers.google.com/web/fundamentals/web-components/best-practices#avoid-reentrancy
if (this.hasOwnProperty(propName)) {
newValue = this[propName];
delete this[propName];
}

this[propName] = newValue === null && typeof this[propName] === 'boolean' ? false : newValue;
});
};
Expand Down