Merge pull request #1384 from Windvis/chore/default-helper-manager-fe…

…ature-flag Add a `DEFAULT_HELPER_MANAGER` feature flag
glimmerjs · Mar 15, 2022 · 2ad32ed · 2ad32ed
2 parents 67e650f + d1734cd
commit 2ad32ed
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 28 deletions.
diff --git a/packages/@glimmer/global-context/index.ts b/packages/@glimmer/global-context/index.ts
@@ -17,6 +17,10 @@ import { DEBUG } from '@glimmer/env';
 
 //////////
 
+export let FEATURE_DEFAULT_HELPER_MANAGER = true;
+
+//////////
+
 /**
  * Interfaces
  *
@@ -154,6 +158,9 @@ export interface GlobalContext {
       id: string;
     }
   ) => void;
+  FEATURES?: {
+    DEFAULT_HELPER_MANAGER?: boolean;
+  };
 }
 
 let globalContextWasSet = false;
@@ -179,6 +186,10 @@ export default function setGlobalContext(context: GlobalContext) {
   warnIfStyleNotTrusted = context.warnIfStyleNotTrusted;
   assert = context.assert;
   deprecate = context.deprecate;
+
+  if (typeof context.FEATURES?.DEFAULT_HELPER_MANAGER === 'boolean') {
+    FEATURE_DEFAULT_HELPER_MANAGER = context.FEATURES.DEFAULT_HELPER_MANAGER;
+  }
 }
 
 export let assertGlobalContextWasSet: (() => void) | undefined;

diff --git a/packages/@glimmer/integration-tests/test/managers/helper-manager-test.ts b/packages/@glimmer/integration-tests/test/managers/helper-manager-test.ts
@@ -10,6 +10,9 @@ import {
   TestHelperManager,
 } from '../..';
 import { Arguments, Owner } from '@glimmer/interfaces';
+import { FEATURE_DEFAULT_HELPER_MANAGER } from '@glimmer/global-context';
+
+const SKIP_DEFAULT_HELPER_MANAGER_TESTS = !FEATURE_DEFAULT_HELPER_MANAGER;
 
 class HelperManagerTest extends RenderTest {
   static suiteName = 'Helper Managers';
@@ -32,7 +35,8 @@ class HelperManagerTest extends RenderTest {
     this.assertHTML('hello');
   }
 
-  @test '(Default Helper Manager) plain functions work as helpers'(assert: Assert) {
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain functions work as helpers'(assert: Assert) {
     let count = 0;
 
     const hello = () => {
@@ -53,7 +57,8 @@ class HelperManagerTest extends RenderTest {
     this.assertHTML('plain function');
   }
 
-  @test '(Default Helper Manager) plain functions track positional args'(assert: Assert) {
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain functions track positional args'(assert: Assert) {
     let count = 0;
 
     let obj = (x: string) => {
@@ -80,7 +85,8 @@ class HelperManagerTest extends RenderTest {
     this.assertHTML('there');
   }
 
-  @test '(Default Helper Manager) plain functions entangle with any tracked data'(assert: Assert) {
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain functions entangle with any tracked data'(assert: Assert) {
     let count = 0;
     let trackedState = trackedObj({ value: 'hello' });
 
@@ -100,7 +106,8 @@ class HelperManagerTest extends RenderTest {
     assert.equal(count, 2, 'rendered twice');
   }
 
-  @test '(Default Helper Manager) plain functions do not track unused named args'(assert: Assert) {
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain functions do not track unused named args'(assert: Assert) {
     let count = 0;
 
     let obj = (x: string, _options: Record<string, unknown>) => {
@@ -120,7 +127,8 @@ class HelperManagerTest extends RenderTest {
     this.assertHTML('hello');
   }
 
-  @test '(Default Helper Manager) plain functions tracked used named args'(assert: Assert) {
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain functions tracked used named args'(assert: Assert) {
     let count = 0;
 
     let obj = (_x: string, options: Record<string, unknown>) => {
@@ -141,7 +149,8 @@ class HelperManagerTest extends RenderTest {
     this.assertHTML('there');
   }
 
-  @test '(Default Helper Manager) plain function helpers can have default values (missing data)'(
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain function helpers can have default values (missing data)'(
     assert: Assert
   ) {
     let count = 0;
@@ -157,7 +166,8 @@ class HelperManagerTest extends RenderTest {
     assert.equal(count, 1, 'rendered once');
   }
 
-  @test '(Default Helper Manager) plain function helpers can have overwritten default values'(
+  @test({ skip: SKIP_DEFAULT_HELPER_MANAGER_TESTS })
+  '(Default Helper Manager) plain function helpers can have overwritten default values'(
     assert: Assert
   ) {
     let count = 0;

diff --git a/packages/@glimmer/manager/lib/internal/index.ts b/packages/@glimmer/manager/lib/internal/index.ts
@@ -1,5 +1,6 @@
 import { DEBUG } from '@glimmer/env';
 import { debugToString, _WeakSet } from '@glimmer/util';
+import { FEATURE_DEFAULT_HELPER_MANAGER } from '@glimmer/global-context';
 import {
   InternalComponentManager,
   InternalModifierManager,
@@ -143,10 +144,12 @@ export function getInternalHelperManager(
 
   let manager = getManager(HELPER_MANAGERS, definition);
 
-  // Functions are special-cased because functions are defined
-  // as the "default" helper, per: https://github.com/emberjs/rfcs/pull/756
-  if (manager === undefined && typeof definition === 'function') {
-    manager = DEFAULT_MANAGER;
+  if (FEATURE_DEFAULT_HELPER_MANAGER) {
+    // Functions are special-cased because functions are defined
+    // as the "default" helper, per: https://github.com/emberjs/rfcs/pull/756
+    if (manager === undefined && typeof definition === 'function') {
+      manager = DEFAULT_MANAGER;
+    }
   }
 
   if (manager) {

diff --git a/packages/@glimmer/manager/package.json b/packages/@glimmer/manager/package.json
@@ -3,11 +3,12 @@
   "version": "0.84.0",
   "repository": "https://github.com/glimmerjs/glimmer-vm/tree/master/packages/@glimmer/program",
   "dependencies": {
+    "@glimmer/destroyable": "0.84.0",
     "@glimmer/env": "0.1.7",
+    "@glimmer/global-context": "0.84.0",
     "@glimmer/interfaces": "0.84.0",
-    "@glimmer/destroyable": "0.84.0",
     "@glimmer/reference": "0.84.0",
-    "@glimmer/validator": "0.84.0",
-    "@glimmer/util": "0.84.0"
+    "@glimmer/util": "0.84.0",
+    "@glimmer/validator": "0.84.0"
   }
 }
diff --git a/packages/@glimmer/manager/test/managers-test.ts b/packages/@glimmer/manager/test/managers-test.ts
@@ -1,4 +1,5 @@
 import { DEBUG } from '@glimmer/env';
+import { FEATURE_DEFAULT_HELPER_MANAGER } from '@glimmer/global-context';
 import {
   ComponentManager,
   HelperManager,
@@ -188,20 +189,22 @@ module('Managers', () => {
       assert.equal(instance['factory'], factory, 'manager has correct delegate factory');
     });
 
-    test('it determines the default manager', (assert) => {
-      let myTestHelper = () => 0;
-      let instance = getInternalHelperManager(myTestHelper) as CustomHelperManager<object>;
-
-      assert.ok(typeof instance === 'object', 'manager is an internal manager');
-      assert.ok(
-        typeof instance.getHelper({}) === 'function',
-        'manager can generate helper function'
-      );
-      assert.strictEqual(
-        instance['factory']({})?.getDebugName?.(myTestHelper),
-        '(helper function myTestHelper)'
-      );
-    });
+    if (FEATURE_DEFAULT_HELPER_MANAGER) {
+      test('it determines the default manager', (assert) => {
+        let myTestHelper = () => 0;
+        let instance = getInternalHelperManager(myTestHelper) as CustomHelperManager<object>;
+
+        assert.ok(typeof instance === 'object', 'manager is an internal manager');
+        assert.ok(
+          typeof instance.getHelper({}) === 'function',
+          'manager can generate helper function'
+        );
+        assert.strictEqual(
+          instance['factory']({})?.getDebugName?.(myTestHelper),
+          '(helper function myTestHelper)'
+        );
+      });
+    }
 
     test('it works with internal helpers', (assert) => {
       let helper = () => {