Merge pull request #17884 from emberjs/upgrade-glimmer-release

Backport Glimmer VM upgrade + bugfixes to release

Co-authored-by: Edward Faulkner <[email protected]>

CHANGELOG.md

@@ -3,6 +3,7 @@
 
 ### v3.9.1 (UNRELEASED)
 
+- [#17870](https://github.com/emberjs/ember.js/pull/17870) / [#17871](https://github.com/emberjs/ember.js/pull/17871) [BUGFIX] Fix issue where `...attributes` may incorrectly overwrite attributes unexpectedly, depending on its position.
 - [#17874](https://github.com/emberjs/ember.js/pull/17874) [BUGFIX] Fix issue with `event.stopPropagation()` in component event handlers when jQuery is disabled.
 - [#17876](https://github.com/emberjs/ember.js/pull/17876) [BUGFIX] Fix issue with multiple `{{action}}` modifiers on the same element when jQuery is disabled.
 

package.json

@@ -89,14 +89,14 @@
     "@babel/plugin-transform-shorthand-properties": "^7.2.0",
     "@babel/plugin-transform-spread": "^7.2.2",
     "@babel/plugin-transform-template-literals": "^7.2.0",
-    "@glimmer/compiler": "^0.37.1",
+    "@glimmer/compiler": "^0.38.4",
     "@glimmer/env": "^0.1.7",
-    "@glimmer/interfaces": "^0.37.1",
-    "@glimmer/node": "^0.37.1",
-    "@glimmer/opcode-compiler": "^0.37.1",
-    "@glimmer/program": "^0.37.1",
-    "@glimmer/reference": "^0.37.1",
-    "@glimmer/runtime": "^0.37.1",
+    "@glimmer/interfaces": "^0.38.4",
+    "@glimmer/node": "^0.38.4",
+    "@glimmer/opcode-compiler": "^0.38.4",
+    "@glimmer/program": "^0.38.4",
+    "@glimmer/reference": "^0.38.4",
+    "@glimmer/runtime": "^0.38.4",
     "@types/qunit": "^2.5.4",
     "@types/rsvp": "^4.0.2",
     "auto-dist-tag": "^1.0.0",
@@ -115,7 +115,7 @@
     "broccoli-rollup": "^2.1.1",
     "broccoli-source": "^1.1.0",
     "broccoli-string-replace": "^0.1.2",
-    "broccoli-typescript-compiler": "^4.0.1",
+    "broccoli-typescript-compiler": "^4.1.0",
     "broccoli-uglify-sourcemap": "^2.2.0",
     "common-tags": "^1.8.0",
     "core-js": "^2.6.5",

packages/@ember/-internals/glimmer/lib/helpers/loc.ts

@@ -38,5 +38,5 @@ import { helper } from '../helper';
   @public
 */
 export default helper(function(params) {
-  return loc.apply(null, params);
+  return loc.apply(null, params as any /* let the other side handle errors */);
 });

packages/@ember/-internals/glimmer/lib/utils/references.ts

@@ -21,7 +21,7 @@ import {
   ConditionalReference as GlimmerConditionalReference,
   PrimitiveReference,
 } from '@glimmer/runtime';
-import { Option } from '@glimmer/util';
+import { Option, unreachable } from '@glimmer/util';
 import { HelperFunction, HelperInstance, RECOMPUTE_TAG } from '../helper';
 import emberToBool from './to-bool';
 
@@ -481,14 +481,56 @@ export function referenceFromParts(
   return reference;
 }
 
+type Primitive = undefined | null | boolean | number | string;
+
+function isObject(value: Opaque): value is object {
+  return value !== null && typeof value === 'object';
+}
+
+function isFunction(value: Opaque): value is Function {
+  return typeof value === 'function';
+}
+
+function isPrimitive(value: Opaque): value is Primitive {
+  if (DEBUG) {
+    let type = typeof value;
+    return (
+      value === undefined ||
+      value === null ||
+      type === 'boolean' ||
+      type === 'number' ||
+      type === 'string'
+    );
+  } else {
+    return true;
+  }
+}
+
 export function valueToRef<T = Opaque>(value: T, bound = true): VersionedPathReference<T> {
-  if (value !== null && typeof value === 'object') {
+  if (isObject(value)) {
     // root of interop with ember objects
     return bound ? new RootReference(value) : new UnboundReference(value);
-  }
-  // ember doesn't do observing with functions
-  if (typeof value === 'function') {
+  } else if (isFunction(value)) {
+    // ember doesn't do observing with functions
     return new UnboundReference(value);
+  } else if (isPrimitive(value)) {
+    return PrimitiveReference.create(value);
+  } else if (DEBUG) {
+    let type = typeof value;
+    let output: Option<string>;
+
+    try {
+      output = String(value);
+    } catch (e) {
+      output = null;
+    }
+
+    if (output) {
+      throw unreachable(`[BUG] Unexpected ${type} (${output})`);
+    } else {
+      throw unreachable(`[BUG] Unexpected ${type}`);
+    }
+  } else {
+    throw unreachable();
   }
-  return PrimitiveReference.create(value);
 }

packages/@ember/-internals/glimmer/lib/utils/serialization-first-node-helpers.ts

@@ -1 +1 @@
-export { isSerializationFirstNode } from '@glimmer/util';
+export { isSerializationFirstNode } from '@glimmer/runtime';

packages/@ember/-internals/glimmer/tests/integration/components/angle-bracket-invocation-test.js

@@ -1,6 +1,7 @@
 import { moduleFor, RenderingTestCase, strip, classes, runTask } from 'internal-test-helpers';
 
 import { EMBER_GLIMMER_ANGLE_BRACKET_INVOCATION } from '@ember/canary-features';
+import { ENV } from '@ember/-internals/environment';
 import { set } from '@ember/-internals/metal';
 
 import { Component } from '../../utils/helpers';
@@ -817,6 +818,59 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_INVOCATION) {
         });
       }
 
+      '@test merges trailing class attribute with `...attributes` in tagless component ("splattributes")'() {
+        let instance;
+        this.registerComponent('foo-bar', {
+          ComponentClass: Component.extend({
+            tagName: '',
+            init() {
+              instance = this;
+              this._super(...arguments);
+              this.localProp = 'qux';
+            },
+          }),
+          template: '<div ...attributes class={{localProp}}>hello</div>',
+        });
+
+        this.render('<FooBar class={{bar}} />', { bar: 'bar' });
+
+        this.assertElement(this.firstChild, {
+          tagName: 'div',
+          attrs: { class: classes('bar qux') },
+          content: 'hello',
+        });
+
+        runTask(() => this.rerender());
+
+        this.assertElement(this.firstChild, {
+          tagName: 'div',
+          attrs: { class: classes('bar qux') },
+          content: 'hello',
+        });
+
+        runTask(() => {
+          set(this.context, 'bar', undefined);
+          set(instance, 'localProp', 'QUZ');
+        });
+
+        this.assertElement(this.firstChild, {
+          tagName: 'div',
+          attrs: { class: classes('QUZ') },
+          content: 'hello',
+        });
+
+        runTask(() => {
+          set(this.context, 'bar', 'bar');
+          set(instance, 'localProp', 'qux');
+        });
+
+        this.assertElement(this.firstChild, {
+          tagName: 'div',
+          attrs: { class: classes('bar qux') },
+          content: 'hello',
+        });
+      }
+
       '@test merges class attribute with `...attributes` in yielded contextual component ("splattributes")'() {
         this.registerComponent('foo-bar', {
           ComponentClass: Component.extend({ tagName: '' }),
@@ -836,6 +890,25 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_INVOCATION) {
         });
       }
 
+      '@test merges trailing class attribute with `...attributes` in yielded contextual component ("splattributes")'() {
+        this.registerComponent('foo-bar', {
+          ComponentClass: Component.extend({ tagName: '' }),
+          template: '{{yield (hash baz=(component "foo-bar/baz"))}}',
+        });
+        this.registerComponent('foo-bar/baz', {
+          ComponentClass: Component.extend({ tagName: '' }),
+          template: '<div ...attributes class="default-class" >hello</div>',
+        });
+
+        this.render('<FooBar as |fb|><fb.baz class="custom-class" title="foo"></fb.baz></FooBar>');
+
+        this.assertElement(this.firstChild, {
+          tagName: 'div',
+          attrs: { class: classes('custom-class default-class'), title: 'foo' },
+          content: 'hello',
+        });
+      }
+
       '@test the attributes passed on invocation trump over the default ones on elements with `...attributes` in yielded contextual component ("splattributes")'() {
         this.registerComponent('foo-bar', {
           ComponentClass: Component.extend({ tagName: '' }),
@@ -1020,4 +1093,86 @@ if (EMBER_GLIMMER_ANGLE_BRACKET_INVOCATION) {
       }
     }
   );
+
+  moduleFor(
+    'AngleBracket Invocation (splattributes)',
+    class extends RenderingTestCase {
+      constructor() {
+        super(...arguments);
+        this._TEMPLATE_ONLY_GLIMMER_COMPONENTS = ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS;
+        ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS = true;
+      }
+
+      teardown() {
+        super.teardown();
+        ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS = this._TEMPLATE_ONLY_GLIMMER_COMPONENTS;
+      }
+
+      registerComponent(name, template) {
+        super.registerComponent(name, { template, ComponentClass: null });
+      }
+
+      '@test angle bracket invocation can pass merge ...attributes'() {
+        this.registerComponent(
+          'qux',
+          '<div data-from-qux-before ...attributes data-from-qux-after></div>'
+        );
+        this.registerComponent(
+          'bar',
+          '<Qux data-from-bar-before ...attributes data-from-bar-after />'
+        );
+        this.registerComponent(
+          'foo',
+          '<Bar data-from-foo-before ...attributes data-from-foo-after />'
+        );
+
+        this.render('<Foo data-from-top />');
+        this.assertHTML(`<div
+          data-from-qux-before=""
+          data-from-bar-before=""
+          data-from-foo-before=""
+          data-from-top=""
+          data-from-foo-after=""
+          data-from-bar-after=""
+          data-from-qux-after=""
+        ></div>`);
+      }
+
+      '@test angle bracket invocation can allow invocation side to override attributes with ...attributes'() {
+        this.registerComponent('qux', '<div id="qux" ...attributes />');
+        this.registerComponent('bar', '<Qux id="bar" ...attributes />');
+        this.registerComponent('foo', '<Bar id="foo" ...attributes />');
+
+        this.render('<Foo id="top" />');
+        this.assertHTML('<div id="top"></div>');
+      }
+
+      '@test angle bracket invocation can override invocation side attributes with ...attributes'() {
+        this.registerComponent('qux', '<div ...attributes id="qux" />');
+        this.registerComponent('bar', '<Qux ...attributes id="bar" />');
+        this.registerComponent('foo', '<Bar ...attributes id="foo" />');
+
+        this.render('<Foo id="top" />');
+        this.assertHTML('<div id="qux"></div>');
+      }
+
+      '@test angle bracket invocation can forward classes before ...attributes to a nested component'() {
+        this.registerComponent('qux', '<div class="qux" ...attributes />');
+        this.registerComponent('bar', '<Qux class="bar" ...attributes />');
+        this.registerComponent('foo', '<Bar class="foo" ...attributes />');
+
+        this.render('<Foo class="top" />');
+        this.assertHTML('<div class="qux bar foo top"></div>');
+      }
+
+      '@test angle bracket invocation can forward classes after ...attributes to a nested component'() {
+        this.registerComponent('qux', '<div ...attributes class="qux" />');
+        this.registerComponent('bar', '<Qux ...attributes class="bar" />');
+        this.registerComponent('foo', '<Bar ...attributes class="foo" />');
+
+        this.render('<Foo class="top" />');
+        this.assertHTML('<div class="top foo bar qux"></div>');
+      }
+    }
+  );
 }

packages/@ember/-internals/metal/lib/computed.ts

@@ -23,7 +23,7 @@ import { tagForProperty, update } from './tags';
 import { getCurrentTracker, setCurrentTracker } from './tracked';
 
 export type ComputedPropertyGetter = (keyName: string) => any;
-export type ComputedPropertySetter = (keyName: string, value: any) => any;
+export type ComputedPropertySetter = (keyName: string, value: any, cachedValue?: any) => any;
 
 export interface ComputedPropertyGetterAndSetter {
   get?: ComputedPropertyGetter;

packages/@ember/-internals/metal/lib/transaction.ts

@@ -170,9 +170,9 @@ if (DEBUG) {
 
   let runner = new TransactionRunner();
 
-  runInTransaction = runner.runInTransaction.bind(runner);
-  didRender = runner.didRender.bind(runner);
-  assertNotRendered = runner.assertNotRendered.bind(runner);
+  runInTransaction = (...args) => runner.runInTransaction(...args);
+  didRender = (...args) => runner.didRender(...args);
+  assertNotRendered = (...args) => runner.assertNotRendered(...args);
 } else {
   // in production do nothing to detect reflushes
   runInTransaction = <T extends object, K extends MethodKey<T>>(context: T, methodName: K) => {

packages/@ember/-internals/routing/lib/location/auto_location.ts

@@ -243,7 +243,7 @@ function detectImplementation(options: DetectionOptions) {
     if (currentPath === historyPath) {
       implementation = 'history';
     } else if (currentPath.substr(0, 2) === '/#') {
-      history!.replaceState({ path: historyPath }, undefined, historyPath);
+      history!.replaceState({ path: historyPath }, '', historyPath);
       implementation = 'history';
     } else {
       cancelRouterSetup = true;