Remove useDefaultValueIfUndefined optional feature, amke it default b…

…ehavior As discussed in ember-bootstrap#1282 (comment), we remove the optional feature introduced in ember-bootstrap#1248 again, and make its behavior the default one.
CloudTomatoes · Nov 13, 2020 · d3d4708 · d3d4708
1 parent f808161
commit d3d4708
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 139 deletions.
diff --git a/addon/components/bs-form.js b/addon/components/bs-form.js
@@ -158,11 +158,11 @@ export default class Form extends Component {
    * Moreover the form's `submit` event handler will validate the model and deny submitting if the model is not validated successfully.
    *
    * @property model
-   * @type Ember.Object
+   * @type object
    * @public
    */
   @defaultValue
-  model = macroCondition(getOwnConfig().useDefaultValueIfUndefined) ? {} : undefined;
+  model = {};
 
   /**
    * Set the layout of the form to either "vertical", "horizontal" or "inline". See http://getbootstrap.com/css/#forms-inline and http://getbootstrap.com/css/#forms-horizontal

diff --git a/addon/utils/decorators/arg.js b/addon/utils/decorators/arg.js
@@ -1,16 +1,10 @@
-import { macroCondition, getOwnConfig } from '@embroider/macros';
-
 export default function arg(target, key, descriptor) {
   let defaultValue = descriptor.initializer ? descriptor.initializer() : undefined;
 
   return {
     get() {
-      if (macroCondition(getOwnConfig().useDefaultValueIfUndefined)) {
-        const argValue = this.args[key];
-        return argValue !== undefined ? argValue : defaultValue;
-      } else {
-        return Object.keys(this.args).includes(key) ? this.args[key] : defaultValue;
-      }
+      const argValue = this.args[key];
+      return argValue !== undefined ? argValue : defaultValue;
     },
   };
 }
diff --git a/addon/utils/default-decorator.js b/addon/utils/default-decorator.js
@@ -1,5 +1,4 @@
 import { computed } from '@ember/object';
-import { macroCondition, getOwnConfig } from '@embroider/macros';
 
 export default function defaultValue(target, key, descriptor) {
   let { initializer, value } = descriptor;
@@ -9,15 +8,11 @@ export default function defaultValue(target, key, descriptor) {
       return initializer ? initializer.call(this) : value;
     },
     set(_, v) {
-      if (macroCondition(getOwnConfig().useDefaultValueIfUndefined)) {
-        if (v !== undefined) {
-          return v;
-        }
-
-        return initializer ? initializer.call(this) : value;
-      } else {
+      if (v !== undefined) {
         return v;
       }
+
+      return initializer ? initializer.call(this) : value;
     },
   })(target, key, { ...descriptor, value: undefined, initializer: undefined });
 }
diff --git a/config/ember-try.js b/config/ember-try.js
@@ -130,17 +130,6 @@ module.exports = async function () {
           FASTBOOT_DISABLED: true,
         },
       }),
-      {
-        name: 'optional-feature-use-default-value-if-undefined',
-        npm: {
-          devDependencies: {
-            bootstrap: bootstrapVersion,
-          },
-        },
-        env: {
-          OPTIONAL_FEATURE_USE_DEFAULT_VALUE_IF_UNDEFINED: true,
-        },
-      },
     ],
   };
 };
diff --git a/ember-cli-build.js b/ember-cli-build.js
@@ -29,10 +29,6 @@ module.exports = function (defaults) {
     },
   };
 
-  if (process.env.OPTIONAL_FEATURE_USE_DEFAULT_VALUE_IF_UNDEFINED) {
-    options['ember-bootstrap'].useDefaultValueIfUndefined = true;
-  }
-
   let app = new EmberAddon(defaults, options);
 
   /*

diff --git a/index.js b/index.js
@@ -19,7 +19,6 @@ const defaultOptions = {
   importBootstrapFont: false,
   insertEmberWormholeElementToDom: true,
   bootstrapVersion: 4,
-  useDefaultValueIfUndefined: false,
 };
 
 const supportedPreprocessors = ['less', 'sass'];
@@ -99,9 +98,6 @@ module.exports = {
     // setup config for @embroider/macros
     this.options['@embroider/macros'].setOwnConfig.isBS3 = this.getBootstrapVersion() === 3;
     this.options['@embroider/macros'].setOwnConfig.isBS4 = this.getBootstrapVersion() === 4;
-    this.options['@embroider/macros'].setOwnConfig.useDefaultValueIfUndefined = Boolean(
-      this.bootstrapOptions.useDefaultValueIfUndefined
-    );
   },
 
   options: {

diff --git a/tests/helpers/bootstrap.js b/tests/helpers/bootstrap.js
@@ -1,6 +1,5 @@
 import config from 'dummy/config/environment';
-import { dependencySatisfies, getConfig, getOwnConfig, macroCondition } from '@embroider/macros';
-import { module, skip, test } from 'qunit';
+import { skip, test } from 'qunit';
 import { Promise } from 'rsvp';
 
 const currentBootstrapVersion = parseInt(config.bootstrapVersion);
@@ -29,37 +28,6 @@ export function versionDependent(v3, v4) {
   return v4;
 }
 
-// eslint-disable-next-line ember/no-test-module-for
-export function moduleForOptionalFeature(optionalFeature, fn) {
-  let optionalFeatureEnabled;
-
-  // `getConfig('ember-bootstrap')` is the correct way to retrieve the
-  // configuration of the addon it it's dummy app. But the `getConfig`
-  // macro is broken for dummy app of an addon in classic builds.
-  // `getOwnConfig` macro also has a bug in classic builds. It resolves
-  // with the configuration of the addon not of it's dummy app. We can
-  // use that bug as a work-a-round for classic builds until the but of
-  // `getConfig` macro has been fixed.
-  // Both bugs are tracked in this issue:
-  // https://github.com/embroider-build/embroider/issues/537
-  if (macroCondition(dependencySatisfies('@embroider/webpack', '*'))) {
-    optionalFeatureEnabled = getConfig('ember-bootstrap')[optionalFeature];
-  } else {
-    optionalFeatureEnabled = getOwnConfig()[optionalFeature];
-  }
-
-  if (optionalFeatureEnabled === undefined) {
-    throw new Error(`Optional feature ${optionalFeature} does not exist.`);
-  }
-
-  let name = `Optional feature: ${optionalFeature}`;
-  if (optionalFeatureEnabled) {
-    module(name, fn);
-  } else {
-    skip(name, fn);
-  }
-}
-
 export function visibilityClass() {
   return versionDependent('in', 'show');
 }

diff --git a/tests/integration/components/bs-form-test.js b/tests/integration/components/bs-form-test.js
@@ -1,37 +1,34 @@
 import EmberObject from '@ember/object';
 import Component from '@ember/component';
 import { A } from '@ember/array';
-import { resolve, reject } from 'rsvp';
+import RSVP, { defer, reject, resolve } from 'rsvp';
 import { module } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import {
-  render,
+  blur,
   click,
   fillIn,
-  triggerKeyEvent,
+  focus,
+  render,
+  settled,
   triggerEvent,
+  triggerKeyEvent,
   waitFor,
   waitUntil,
-  settled,
-  focus,
-  blur,
 } from '@ember/test-helpers';
 import {
   formFeedbackClass,
-  moduleForOptionalFeature,
+  formFeedbackElement,
   test,
-  testRequiringFocus,
   testBS3,
   testBS4,
+  testRequiringFocus,
   validationErrorClass,
-  formFeedbackElement,
   validationSuccessClass,
 } from '../../helpers/bootstrap';
 import hbs from 'htmlbars-inline-precompile';
-import { defer } from 'rsvp';
 import { next, run } from '@ember/runloop';
 import setupNoDeprecations from '../../helpers/setup-no-deprecations';
-import RSVP from 'rsvp';
 import a11yAudit from 'ember-a11y-testing/test-support/audit';
 import sinon from 'sinon';
 
@@ -1068,89 +1065,86 @@ module('Integration | Component | bs-form', function (hooks) {
     await triggerEvent('form', 'submit');
   });
 
-  // eslint-disable-next-line ember/no-test-module-for
-  moduleForOptionalFeature('useDefaultValueIfUndefined', function () {
-    test('it uses default value false for disabled argument if undefined', async function (assert) {
-      this.set('disabled', undefined);
-      await render(
-        hbs`
+  test('it uses default value false for disabled argument if undefined', async function (assert) {
+    this.set('disabled', undefined);
+    await render(
+      hbs`
           <BsForm @disabled={{this.disabled}} as |form|>
             <form.element />
           </BsForm>`
-      );
-      assert.dom('input').doesNotHaveAttribute('disabled');
-    });
+    );
+    assert.dom('input').doesNotHaveAttribute('disabled');
+  });
 
-    testBS3('it uses default value for formLayout argument if undefined', async function (assert) {
-      this.set('formLayout', undefined);
-      await render(hbs`
+  testBS3('it uses default value for formLayout argument if undefined', async function (assert) {
+    this.set('formLayout', undefined);
+    await render(hbs`
         <BsForm @formLayout={{this.formLayout}} />
       `);
-      assert.dom('form').hasClass('form');
-    });
+    assert.dom('form').hasClass('form');
+  });
 
-    testBS4('it uses default value for formLayout argument if undefined', async function (assert) {
-      this.set('formLayout', undefined);
-      await render(hbs`
+  testBS4('it uses default value for formLayout argument if undefined', async function (assert) {
+    this.set('formLayout', undefined);
+    await render(hbs`
         <BsForm @formLayout={{this.formLayout}} />
       `);
-      assert.dom('form').doesNotHaveAttribute('class');
-    });
+    assert.dom('form').doesNotHaveAttribute('class');
+  });
 
-    test('it uses a POJO as model if undefined', async function (assert) {
-      this.set('model', undefined);
-      this.set('submitHandler', (model) => {
-        assert.step('submit action on untouched form');
-        assert.ok(typeof model === 'object');
-      });
-      await render(hbs`
+  test('it uses a POJO as model if undefined', async function (assert) {
+    this.set('model', undefined);
+    this.set('submitHandler', (model) => {
+      assert.step('submit action on untouched form');
+      assert.ok(typeof model === 'object');
+    });
+    await render(hbs`
         <BsForm @model={{this.model}} @onSubmit={{this.submitHandler}} as |form|>
           <form.element @property="name" />
         </BsForm>
       `);
-      await triggerEvent('form', 'submit');
-      assert.verifySteps(['submit action on untouched form']);
+    await triggerEvent('form', 'submit');
+    assert.verifySteps(['submit action on untouched form']);
 
-      this.set('submitHandler', (model) => {
-        assert.step('submit action after user input');
-        assert.deepEqual(model, { name: 'John Doe' });
-      });
-      await fillIn('input', 'John Doe');
-      await triggerEvent('form', 'submit');
-      assert.verifySteps(['submit action after user input']);
+    this.set('submitHandler', (model) => {
+      assert.step('submit action after user input');
+      assert.deepEqual(model, { name: 'John Doe' });
     });
+    await fillIn('input', 'John Doe');
+    await triggerEvent('form', 'submit');
+    assert.verifySteps(['submit action after user input']);
+  });
 
-    test('POJO created for model if undefined does not leak between two instances', async function (assert) {
-      this.set('model', undefined);
-      this.set('submitHandlerForFirstForm', (model) => {
-        assert.step('first form submitted');
-        assert.deepEqual(model, { name: 'John Doe' }, 'name is set on model for first form');
-      });
-      this.set('submitHandlerForSecondForm', (model) => {
-        assert.step('second form submitted');
-        assert.deepEqual(model, {}, 'name is not set on model for second form');
-      });
-      await render(hbs`
+  test('POJO created for model if undefined does not leak between two instances', async function (assert) {
+    this.set('model', undefined);
+    this.set('submitHandlerForFirstForm', (model) => {
+      assert.step('first form submitted');
+      assert.deepEqual(model, { name: 'John Doe' }, 'name is set on model for first form');
+    });
+    this.set('submitHandlerForSecondForm', (model) => {
+      assert.step('second form submitted');
+      assert.deepEqual(model, {}, 'name is not set on model for second form');
+    });
+    await render(hbs`
         <BsForm @model={{this.model}} @onSubmit={{this.submitHandlerForFirstForm}} data-test-first-form as |form|>
           <form.element @property="name" />
         </BsForm>
         <BsForm @model={{this.model}} @onSubmit={{this.submitHandlerForSecondForm}} data-test-second-form />
       `);
-      await fillIn('input', 'John Doe');
-      await triggerEvent('[data-test-first-form]', 'submit');
-      await triggerEvent('[data-test-second-form]', 'submit');
-      assert.verifySteps(['first form submitted', 'second form submitted']);
-    });
+    await fillIn('input', 'John Doe');
+    await triggerEvent('[data-test-first-form]', 'submit');
+    await triggerEvent('[data-test-second-form]', 'submit');
+    assert.verifySteps(['first form submitted', 'second form submitted']);
+  });
 
-    test('it uses default value false for readonly argument if undefined', async function (assert) {
-      this.set('readonly', undefined);
-      await render(
-        hbs`
+  test('it uses default value false for readonly argument if undefined', async function (assert) {
+    this.set('readonly', undefined);
+    await render(
+      hbs`
           <BsForm @readonly={{this.readonly}} as |form|>
             <form.element />
           </BsForm>`
-      );
-      assert.dom('input').doesNotHaveAttribute('readonly');
-    });
+    );
+    assert.dom('input').doesNotHaveAttribute('readonly');
   });
 });