Backport "Don't run getters while applying mixins" to 4.8 LTS

.github/workflows/ci.yml

@@ -92,7 +92,7 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v2
         with:
-          node-version: 12.x
+          node-version: 14.x
           cache: yarn
       - name: install dependencies
         run: yarn install --frozen-lockfile --non-interactive

packages/@ember/-internals/runtime/tests/mixins/accessor_test.js

@@ -0,0 +1,56 @@
+import EmberObject from '@ember/object';
+import { moduleFor, RenderingTestCase } from 'internal-test-helpers';
+
+moduleFor(
+  'runtime: Mixin Accessors',
+  class extends RenderingTestCase {
+    ['@test works with getters'](assert) {
+      let value = 'building';
+
+      let Base = EmberObject.extend({
+        get foo() {
+          if (value === 'building') {
+            throw Error('base should not be called yet');
+          }
+
+          return "base's foo";
+        },
+      });
+
+      // force Base to be finalized so its properties will contain `foo`
+      Base.proto();
+
+      class Child extends Base {
+        get foo() {
+          if (value === 'building') {
+            throw Error('child should not be called yet');
+          }
+
+          return "child's foo";
+        }
+      }
+
+      Child.proto();
+
+      let Grandchild = Child.extend({
+        get foo() {
+          if (value === 'building') {
+            throw Error('grandchild should not be called yet');
+          }
+
+          return value;
+        },
+      });
+
+      let instance = Grandchild.create();
+
+      value = 'done building';
+
+      assert.equal(instance.foo, 'done building', 'getter defined correctly');
+
+      value = 'changed value';
+
+      assert.equal(instance.foo, 'changed value', 'the value is a real getter, not a snapshot');
+    }
+  }
+);

packages/@ember/array/index.ts

@@ -1930,11 +1930,11 @@ let NativeArray = Mixin.create(MutableArray, Observable, {
 });
 
 // Remove any methods implemented natively so we don't override them
-const ignore = ['length'];
-NativeArray.keys().forEach((methodName) => {
+const ignore: string[] = ['length'];
+NativeArray.keys().forEach((methodName: unknown): void => {
   // SAFETY: It's safe to read unknown properties from an object
-  if ((Array.prototype as any)[methodName]) {
-    ignore.push(methodName);
+  if (Reflect.has(Array.prototype, methodName as string)) {
+    ignore.push(methodName as string);
   }
 });
 
@@ -1963,7 +1963,7 @@ if (ENV.EXTEND_PROTOTYPES.Array) {
 
     if (isEmberArray(arr)) {
       // SAFETY: If it's a true native array and it is also an EmberArray then it should be an Ember NativeArray
-      return arr as NativeArray<T>;
+      return arr as unknown as NativeArray<T>;
     } else {
       // SAFETY: This will return an NativeArray but TS can't infer that.
       return NativeArray.apply(arr ?? []) as NativeArray<T>;

packages/@ember/object/mixin.ts

@@ -1,5 +1,5 @@
 /**
-@module @ember/object/mixin
+@module @ember/object
 */
 import { INIT_FACTORY } from '@ember/-internals/container';
 import type { Meta } from '@ember/-internals/meta';
@@ -10,32 +10,34 @@ import { DEBUG } from '@glimmer/env';
 import { _WeakSet } from '@glimmer/util';
 import type {
   ComputedDecorator,
+  ComputedDescriptor,
   ComputedPropertyGetter,
   ComputedPropertyObj,
   ComputedPropertySetter,
-  ComputedDescriptor,
 } from '@ember/-internals/metal';
 import {
   ComputedProperty,
+  addObserver,
+  defineDecorator,
+  defineValue,
   descriptorForDecorator,
+  isClassicDecorator,
   makeComputedDecorator,
   nativeDescDecorator,
-  setUnprocessedMixins,
-  addObserver,
   removeObserver,
   revalidateObservers,
-  defineDecorator,
-  defineValue,
+  setUnprocessedMixins,
 } from '@ember/-internals/metal';
-import { addListener, removeListener } from '@ember/object/events';
+import { addListener, removeListener } from './events';
 
 const a_concat = Array.prototype.concat;
 const { isArray } = Array;
 
-function extractAccessors(properties: { [key: string]: any } | undefined) {
+function extractAccessors(properties: { [key: string]: unknown } | undefined) {
   if (properties !== undefined) {
     for (let key of Object.keys(properties)) {
-      let desc = Object.getOwnPropertyDescriptor(properties, key)!;
+      let desc = Object.getOwnPropertyDescriptor(properties, key);
+      assert(`a property descriptor for ${key} must exist`, desc !== undefined);
 
       if (desc.get !== undefined || desc.set !== undefined) {
         Object.defineProperty(properties, key, { value: nativeDescDecorator(desc) });
@@ -225,7 +227,7 @@ function mergeMixins(
   keys: string[],
   keysWithSuper: string[]
 ): void {
-  let currentMixin;
+  let currentMixin: MixinLike | undefined;
 
   for (let i = 0; i < mixins.length; i++) {
     currentMixin = mixins[i];
@@ -299,12 +301,17 @@ function mergeProps(
       let desc = meta.peekDescriptors(key);
 
       if (desc === undefined) {
-        // The superclass did not have a CP, which means it may have
-        // observers or listeners on that property.
-        let prev = (values[key] = base[key]);
-
-        if (typeof prev === 'function') {
-          updateObserversAndListeners(base, key, prev, false);
+        // If the value is a classic decorator, we don't want to actually
+        // access it, because that will execute the decorator while we're
+        // building the class.
+        if (!isClassicDecorator(value)) {
+          // The superclass did not have a CP, which means it may have
+          // observers or listeners on that property.
+          let prev = (values[key] = base[key]);
+
+          if (typeof prev === 'function') {
+            updateObserversAndListeners(base, key, prev, false);
+          }
         }
       } else {
         descs[key] = desc;
@@ -534,8 +541,6 @@ export default class Mixin {
   /** @internal */
   _without: any[] | undefined;
 
-  declare [INIT_FACTORY]?: null;
-
   /** @internal */
   constructor(mixins: Mixin[] | undefined, properties?: { [key: string]: any }) {
     MIXINS.add(this);
@@ -546,7 +551,7 @@ export default class Mixin {
 
     if (DEBUG) {
       // Eagerly add INIT_FACTORY to avoid issues in DEBUG as a result of Object.seal(mixin)
-      this[INIT_FACTORY] = null;
+      (this as Record<typeof INIT_FACTORY, unknown>)[INIT_FACTORY] = null;
       /*
         In debug builds, we seal mixins to help avoid performance pitfalls.
 
@@ -738,15 +743,14 @@ function _detect(curMixin: Mixin, targetMixin: Mixin, seen = new Set()): boolean
   return false;
 }
 
-function _keys(mixin: Mixin, ret = new Set<string>(), seen = new Set()) {
+function _keys(mixin: Mixin, ret = new Set(), seen = new Set()) {
   if (seen.has(mixin)) {
     return;
   }
   seen.add(mixin);
 
   if (mixin.properties) {
-    let props = Object.keys(mixin.properties);
-    for (let prop of props) {
+    for (const prop of Object.keys(mixin.properties)) {
       ret.add(prop);
     }
   } else if (mixin.mixins) {
@@ -755,3 +759,5 @@ function _keys(mixin: Mixin, ret = new Set<string>(), seen = new Set()) {
 
   return ret;
 }
+
+export { Mixin };

smoke-tests/ember-test-app/package.json

@@ -63,7 +63,7 @@
     "webpack": "^5.65.0"
   },
   "engines": {
-    "node": "12.* || 14.* || >= 16"
+    "node": "14.* || >= 16"
   },
   "ember": {
     "edition": "octane"