From afbd129be49d636a09c986e97ae85e3f9cf5080c Mon Sep 17 00:00:00 2001
From: Nicholas Hyatt <nicholaspaulhyatt@gmail.com>
Date: Wed, 7 Jul 2021 11:57:55 -0500
Subject: [PATCH] fix(runtime) prevent shadowing on non-upgraded compoents

* fix an issue in the custom elements bundle where setting a property
   on an element programatically before it was upgraded it would shadow
   the accessors
  * this would additionally break any watchers as well.
---
 src/runtime/connected-callback.ts |  2 +-
 src/runtime/proxy-component.ts    | 39 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/runtime/connected-callback.ts b/src/runtime/connected-callback.ts
index 235213fa9d0..715383c9395 100644
--- a/src/runtime/connected-callback.ts
+++ b/src/runtime/connected-callback.ts
@@ -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];
diff --git a/src/runtime/proxy-component.ts b/src/runtime/proxy-component.ts
index d3e276d7dd7..f3b8f95a524 100644
--- a/src/runtime/proxy-component.ts
+++ b/src/runtime/proxy-component.ts
@@ -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;
         });
       };