Merge pull request #44 from alleyinteractive/issue/43

Corrects the domContentLoaded handler for deferred and async scripts

Author: goodguyry

CHANGELOG.md

@@ -1,6 +1,16 @@
 # Change Log
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## 3.2.0
+
+**Added**
+
+* Setting `config.load` to `true` adds the provider function call in place so it is executed as soon as the parent script is parsed and loaded.
+
+**Fixed**
+
+* The `domContentLoaded` function no longer executes its callback more than once
+
 ## 3.1.0
 
 **Added**

README.md

@@ -49,12 +49,12 @@ Component elements are denoted by a `data-component` attribute, the value of whi
 
 **options**: _(Optional)_ - An arbitrary value, typically an object, used by the component. This could be a configuration for another JS library, values used for calculating styles, etc. This is passed to the wrapped function as the `options` property.
 
-**load**: _(Optional)_ - Accepts `false`, array, or a callback. _Default is a `domContentLoaded` callback_.
-
-* `false` will disable loading and instruct `componentProvider` to return the provider function
-* An array, in the format of `[HTMLElement, event]`, will listen on `HTMLElement` for `event` (e.g., `[window, 'load']`)
-* A callback that accepts a function and contains the logic to call the function
+**load**: _(Optional)_ - _Default is a `DOMContentLoaded` handler_.
 
+* `false` - prevents execution and instructs `componentProvider` to return the provider function
+* `true` - Adds the provider function call in place so it is executed as soon as the parent script is parsed and loaded.
+* `array` - `[HTMLElement, event]` - Adds the provider function as a callback for `event` on `HTMLElement` (e.g., `[window, 'load']`).
+* `handler` - A function that accepts a callback and contains the logic to call it.
 ### Component Properties
 
 Components receive an object of component properties as their only argument. These are based on the config and are included automatically by the framework.
@@ -105,12 +105,13 @@ function productDetails({ element, children, options }) { ... }
 const productDetailsConfig = {
   name: 'product-details',
   component: productDetails,
-  // load: false | array | function
+  // load: boolean | array | function
   querySelectorAll: {
     toggles: '.product-details__toggle',
   },
 };
 
+// The component is executed at 'DOMContentLoaded' due to the absense of a `config.load` value.
 componentProvider(productDetailsConfig);
 ```
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "js-component-framework",
-  "version": "3.1.0",
+  "version": "3.2.0",
   "description": "A framework for configuring a JavaScript component and attaching it to a DOM element or collection of DOM elements, simplifying organization of DOM interactions on your website.",
   "main": "lib/index.js",
   "module": "es/index.js",

src/componentLoader.test.js

@@ -10,7 +10,12 @@ const providerFunc = jest.fn();
 
 test('Does not call the provider function with `load:false`', () => {
   componentLoader(providerFunc, false);
-  expect(providerFunc).toHaveBeenCalledTimes(0);
+  expect(providerFunc).not.toHaveBeenCalled();
+});
+
+test('Does not call the provider function with `load:true`', () => {
+  componentLoader(providerFunc, true);
+  expect(providerFunc).not.toHaveBeenCalled();
 });
 
 test('Calls the provider function with the loader function', () => {
@@ -22,22 +27,22 @@ test('Calls the provider function with the loader function', () => {
 
 test('Loads on event for single instance', () => {
   componentLoader(providerFunc, [eventRoot, 'jscf-test-event']);
-  expect(providerFunc).toHaveBeenCalledTimes(0);
+  expect(providerFunc).not.toHaveBeenCalled();
 
   eventRoot.dispatchEvent(new CustomEvent('jscf-test-event'));
   expect(providerFunc).toHaveBeenCalledTimes(1);
 });
 
 test('Fails silently on invalid event config', () => {
   componentLoader(providerFunc, [null, 'jscf-null-test']);
-  expect(providerFunc).toHaveBeenCalledTimes(0);
+  expect(providerFunc).not.toHaveBeenCalled();
 
   eventRoot.dispatchEvent(new CustomEvent('jscf-null-test'));
-  expect(providerFunc).toHaveBeenCalledTimes(0);
+  expect(providerFunc).not.toHaveBeenCalled();
 
   componentLoader(providerFunc, [eventRoot, 123]);
-  expect(providerFunc).toHaveBeenCalledTimes(0);
+  expect(providerFunc).not.toHaveBeenCalled();
 
   eventRoot.dispatchEvent(new CustomEvent('jscf-fail-test'));
-  expect(providerFunc).toHaveBeenCalledTimes(0);
+  expect(providerFunc).not.toHaveBeenCalled();
 });

src/componentProvider.js

@@ -77,12 +77,22 @@ export default function componentProvider(config) {
     componentArgs.forEach((args) => new Component(args));
   };
 
-  if (load !== false) {
-    // Load the provider function.
-    componentLoader(init, load);
+  // Return the provider function for later execution.
+  if (load === false) {
+    return init;
+  }
 
-    return undefined;
+  /*
+   * Call the provider function so it is executed as soon as the document is
+   * parsed and loaded.
+   *
+   * This is a convenience option and is functionally identical to setting
+   * `config.load` to false and calling the provider function later in the script.
+   */
+  if (load === true) {
+    return void init(); // eslint-disable-line no-void
   }
 
-  return init;
+  // Use the handler defined in the `load` config property.
+  return void componentLoader(init, load); // eslint-disable-line no-void
 }

src/componentProvider.test.js

@@ -95,7 +95,7 @@ test('Returns the expected function for single instance when `load:false`', () =
   const config = { ...baseConfig, name: 'test-one', load: false };
 
   const providerFunction = componentProvider(config);
-  expect(config.component).toHaveBeenCalledTimes(0);
+  expect(config.component).not.toHaveBeenCalled();
 
   providerFunction();
   expect(config.component).toHaveBeenCalledTimes(1);
@@ -106,7 +106,8 @@ test('Returns the expected function for multiple instances when `load:false`', (
   const config = { ...baseConfig, name: 'test-two', load: false };
 
   const providerFunction = componentProvider(config);
-  expect(config.component).toHaveBeenCalledTimes(0);
+  expect(typeof providerFunction).toBe('function');
+  expect(config.component).not.toHaveBeenCalled();
 
   providerFunction();
   expect(config.component).toHaveBeenCalledTimes(2);
@@ -115,6 +116,21 @@ test('Returns the expected function for multiple instances when `load:false`', (
   expect(config.component).toHaveBeenNthCalledWith(2, testTwoExpected[1]);
 });
 
+test('Calls the component immediately when `load:true`', () => {
+  const config = { ...baseConfig, name: 'test-one', load: true };
+
+  const providerFunction = componentProvider(config);
+  expect(providerFunction).toBeUndefined();
+  expect(config.component).toHaveBeenCalledTimes(1);
+});
+
+test('Calls the component for multiple instances immediately when `load:true`', () => {
+  const config = { ...baseConfig, name: 'test-two', load: true };
+
+  componentProvider(config);
+  expect(config.component).toHaveBeenCalledTimes(2);
+});
+
 test('Loads on event for single instance', () => {
   const config = {
     ...baseConfig,
@@ -123,7 +139,7 @@ test('Loads on event for single instance', () => {
   };
 
   componentProvider(config);
-  expect(config.component).toHaveBeenCalledTimes(0);
+  expect(config.component).not.toHaveBeenCalled();
 
   eventRoot.dispatchEvent(new CustomEvent('jscf-test-one-event'));
   expect(config.component).toHaveBeenCalledTimes(1);
@@ -138,7 +154,7 @@ test('Loads on event for multiple instances', () => {
   };
 
   componentProvider(config);
-  expect(config.component).toHaveBeenCalledTimes(0);
+  expect(config.component).not.toHaveBeenCalled();
 
   eventRoot.dispatchEvent(new CustomEvent('jscf-test-two-event'));
   expect(config.component).toHaveBeenCalledTimes(2);
@@ -155,10 +171,10 @@ test('Fails silently on `null` event root', () => {
   };
 
   componentProvider(config);
-  expect(config.component).toHaveBeenCalledTimes(0);
+  expect(config.component).not.toHaveBeenCalled();
 
   eventRoot.dispatchEvent(new CustomEvent('jscf-null-test'));
-  expect(config.component).toHaveBeenCalledTimes(0);
+  expect(config.component).not.toHaveBeenCalled();
 });
 
 test('Provides an empty options object when options are undefined', () => {

src/domContentLoaded.js

@@ -1,17 +1,19 @@
+/* eslint consistent-return: ["error", { "treatUndefinedAsUnspecified": true }] */
+
 /**
  * Executes the given callback when DOMContentLoaded is ready.
  *
- * @param {function} cb Callback to execute once DOMContentLoaded completes.
+ * @param {function} callback Callback to execute once DOMContentLoaded completes.
  */
-const domContentLoaded = (cb) => {
+function domContentLoaded(callback) {
   if (
     document.readyState === 'complete'
     || document.readyState === 'interactive'
   ) {
-    cb();
+    return void callback(); // eslint-disable-line no-void
   }
 
-  document.addEventListener('DOMContentLoaded', cb, { once: true });
-};
+  document.addEventListener('DOMContentLoaded', callback, { once: true });
+}
 
 export default domContentLoaded;

src/domContentLoaded.test.js

@@ -0,0 +1,47 @@
+import domContentLoaded from './domContentLoaded';
+
+const providerFunc = jest.fn();
+const addEventListener = jest.fn(() => {});
+
+beforeAll(() => {
+  // Allows setting document.readyState.
+  Object.defineProperty(document, 'readyState', {
+    value: 'loading',
+    writable: true,
+  });
+
+  Object.defineProperty(document, 'addEventListener', {
+    value: addEventListener,
+  });
+});
+
+afterEach(() => jest.clearAllMocks());
+
+test("'loading' readyState uses DOMContentLoaded", () => {
+  domContentLoaded(providerFunc);
+  expect(providerFunc).not.toHaveBeenCalled();
+
+  expect(addEventListener).toHaveBeenCalledWith(
+    'DOMContentLoaded',
+    providerFunc,
+    { once: true },
+  );
+});
+
+test("'complete' readyState calls the function immediately", () => {
+  document.readyState = 'complete';
+
+  domContentLoaded(providerFunc);
+  expect(providerFunc).toHaveBeenCalledTimes(1);
+
+  expect(addEventListener).not.toHaveBeenCalled();
+});
+
+test("'interactive' readyState calls the funciton immediately", () => {
+  document.readyState = 'interactive';
+
+  domContentLoaded(providerFunc);
+  expect(providerFunc).toHaveBeenCalledTimes(1);
+
+  expect(addEventListener).not.toHaveBeenCalled();
+});