Merge pull request #6 from keyanzhang/1.0

Major cleanup for 1.0.0

.eslintrc

@@ -1,28 +1,9 @@
 {
-  "extends": "airbnb/base",
+  "extends": "airbnb-base",
   "env": {
     "browser": true,
     "node": true,
     "es6": true,
-    "mocha": true,
+    "jest": true,
   },
-  "ecmaFeatures": {
-    "generators": true,
-  },
-  "parser": "babel-eslint",
-  "rules": {
-    "babel/generator-star-spacing": 1,
-    "babel/new-cap": 1,
-    "babel/object-shorthand": 1,
-    "babel/arrow-parens": 1,
-    "babel/no-await-in-loop": 1,
-    "array-bracket-spacing": [1, "always", {
-      "singleValue": true,
-      "objectsInArrays": false,
-      "arraysInArrays": true,
-    }],
-  },
-  "plugins": [
-    "babel"
-  ],
 }

.travis.yml

@@ -18,7 +18,4 @@ addons:
       - clang-3.6
       - g++-4.8
 
-before_install:
-  - npm install -g babel-cli
-
-after_success: npm run cov
+after_success: npm run coveralls

CHANGELOG.md

@@ -0,0 +1,6 @@
+## 1.0.0 (February 19, 2017)
+
+* Bugfix: pass along all actions to next middleware in chain. ([@jebeck](https://github.com/jebeck) in [#5](https://github.com/keyanzhang/redux-worker-middleware/pull/5))
+  * It's now important to let the worker return messages that have the `meta.WebWorker` field removed. Since the returned data will be re-dispatched as a new action and be passed through all the middlewares, keeping the `meta.WebWorker` field may cause an infinite loop.
+* Changed from `console.error` to (throw) real errors for incorrect configs ([#6](https://github.com/keyanzhang/redux-worker-middleware/pull/6))
+* Let the middleware grab `dispatch` earlier ([#6](https://github.com/keyanzhang/redux-worker-middleware/pull/6))

README.md

@@ -24,7 +24,7 @@ In case you need, webpack's [worker-loader](https://github.com/webpack/worker-lo
 
 2. To let the workers work, make sure that your action is [FSA compliant](https://github.com/acdlite/flux-standard-action) and the `action.meta.WebWorker` field is truthy. Otherwise, the middleware will just pass the action along.
 
-3. If an action specifies that it needs to be processed by a worker, The middleware will obey the order. Then when the data comes back, it will be passed along to the rest of the middleware chain.
+3. If an action specifies that it needs to be processed by a worker, The middleware will obey the order. Then when the data comes back, it will be re-dispatched as a new action and be passed through all the middlewares (see [#5](https://github.com/keyanzhang/redux-worker-middleware/pull/5)).
 
 ## Demo
 I wrote this middleware as part of https://github.com/keyanzhang/repo.cat, where I need to parse a lot of markdown stuff to HTML at runtime. So the real demo can be found there: the Web Worker related parts live in [`actions/DataFetching.js`](https://github.com/keyanzhang/repo.cat/blob/master/src/actions/DataFetching.js), [`middlewares/worker.js`](https://github.com/keyanzhang/repo.cat/blob/master/src/middlewares/worker.js), and [`workers/GFMParserWorker.js`](https://github.com/keyanzhang/repo.cat/blob/master/src/workers/GFMParserWorker.js).
@@ -33,9 +33,12 @@ A minimal example can be found as below:
 
 Web Worker: `Add1Worker.js`:
 ```javascript
-self.onmessage = ({ data: action }) => { // data should be a FSA compliant action object.
+self.onmessage = ({ data: action }) => { // `data` should be a FSA compliant action object.
   self.postMessage({
-    ...action,
+    type: action.type,
+    // Notice that we remove the `meta.WebWorker` field from the payload.
+    // Since the returned data will be dispatched as a new action and be passed through all the middlewares,
+    // keeping the `meta.WebWorker` field may cause an infinite loop.
     payload: {
       num: action.payload.num + 1,
     },
@@ -48,7 +51,7 @@ ActionCreator:
 export const add1Action = (n) => ({
   type: 'ADD_1',
   meta: {
-    WebWorker: true, // this line specifies that the worker should show up and do the job
+    WebWorker: true, // This line specifies that the worker should show up and do the job
   },
   payload: {
     num: n,
@@ -83,11 +86,11 @@ const createStoreWithMiddleware = applyMiddleware(
 // ... ...
 ```
 
-That's it! Now when you fire an `add1Action`, the worker will show up and do the computation. The result (action) will be passed along to the next middleware.
+That's it! Now when you fire an `add1Action`, the worker will show up and do the computation. The result (action) will be re-dispatched as a new action and be passed through all the middlewares.
 
 ## Notes
 
-For now, we don't really care if you actually pass it a real Worker instance; as long as it look likes a Worker and works like a Worker (i.e., has a `postMessage` method), it _is_ a Worker. The reason behind is that we want to support Web Worker shims in an easy manner. Take a look at the test cases [here](./test/__setup__/workerPolyfill.js).
+For now, we don't really care if you actually pass it a real Worker instance; as long as it look likes a Worker and works like a Worker (i.e., has a `postMessage` method), it _is_ a Worker. The reason behind is that we want to support Web Worker shims in an easy manner.
 
 ## License
 MIT

package.json

@@ -4,17 +4,16 @@
   "description": "Organized Web Workers for Redux",
   "main": "lib/index.js",
   "files": [
-    "lib",
-    "src"
+    "lib"
   ],
   "scripts": {
     "clean": "rimraf lib",
-    "build:babel": "babel src --out-dir lib",
+    "build:babel": "babel src --out-dir lib --ignore __tests__",
     "build": "npm run clean && npm run lint && npm run test && npm run build:babel",
-    "lint": "eslint src test",
-    "test": "NODE_ENV=test mocha",
-    "test:watch": "npm test -- --watch",
-    "cov": "nyc npm test && nyc report --reporter=text-lcov | coveralls",
+    "lint": "eslint src",
+    "test": "jest",
+    "coverage": "jest --coverage",
+    "coveralls": "npm run coverage && cat ./coverage/lcov.info | coveralls",
     "prepublish": "npm run build"
   },
   "repository": {
@@ -43,17 +42,18 @@
   "devDependencies": {
     "babel-cli": "^6.9.0",
     "babel-core": "^6.4.5",
-    "babel-eslint": "^4.1.8",
+    "babel-jest": "^18.0.0",
     "babel-preset-es2015": "^6.3.13",
     "babel-preset-stage-0": "^6.3.13",
     "coveralls": "^2.11.6",
-    "eslint": "^1.10.3",
-    "eslint-config-airbnb": "^5.0.0",
-    "eslint-plugin-babel": "^3.1.0",
-    "expect": "^1.14.0",
-    "jsdom": "^8.0.2",
-    "mocha": "^2.4.5",
-    "nyc": "^5.5.0",
-    "rimraf": "^2.5.1"
+    "eslint": "^3.15.0",
+    "eslint-config-airbnb-base": "^11.1.0",
+    "eslint-plugin-import": "^2.2.0",
+    "jest": "^18.1.0",
+    "rimraf": "^2.6.0"
+  },
+  "jest": {
+    "collectCoverageFrom": ["src/**/*.js"],
+    "testEnvironment": "node"
   }
 }

src/__tests__/index-test.js

@@ -0,0 +1,150 @@
+import createWorkerMiddleware from '..';
+
+class Worker {
+  constructor(fn = x => x) {
+    this.fn = fn;
+
+    this.postMessage = this.postMessage.bind(this);
+    this.onmessage = undefined;
+  }
+
+  postMessage(msg) {
+    if (this.onmessage) {
+      setTimeout(() => {
+        const data = this.fn(msg);
+        this.onmessage({ data });
+      }, 0);
+    }
+  }
+}
+
+describe('createWorkerMiddleware', () => {
+  const actionWithWorker = {
+    type: 'I_USE_WORKER',
+    meta: {
+      WebWorker: true,
+    },
+    payload: {
+      data: 42,
+      category: 'life',
+    },
+  };
+
+  const actionWithoutWorker = {
+    type: 'I_DONT_USE_WORKER',
+    payload: {
+      data: 23,
+      category: 'number',
+    },
+  };
+
+  const dispatch = jest.fn(x => x);
+
+  beforeEach(() => {
+    dispatch.mockClear();
+  });
+
+  it('should throw if `worker` is falsy', () => {
+    expect(() => {
+      createWorkerMiddleware();
+    }).toThrow(
+      '`createWorkerMiddleware` expects a worker instance as the argument. Instead received: undefined',
+    );
+  });
+
+  it('should throw if `worker.postMessage` is falsy', () => {
+    expect(() => {
+      createWorkerMiddleware({});
+    }).toThrow(
+      'The worker instance is expected to have a `postMessage` method.',
+    );
+  });
+
+  it('the worker should be invoked if the action needs it', (done) => {
+    const mockWorkerBehavior = jest.fn();
+    const middleware = createWorkerMiddleware(new Worker(mockWorkerBehavior));
+
+    const next = (action) => {
+      expect(action).toBe(actionWithWorker);
+      setTimeout(() => {
+        expect(mockWorkerBehavior).toHaveBeenCalledTimes(1);
+        expect(mockWorkerBehavior).toHaveBeenCalledWith(actionWithWorker);
+        done();
+      }, 10);
+    };
+
+    middleware({ dispatch })(next)(actionWithWorker);
+  });
+
+  it('the worker shouldn\'t be invoked if the action doesn\'t need it', (done) => {
+    const mockWorkerBehavior = jest.fn();
+    const middleware = createWorkerMiddleware(new Worker(mockWorkerBehavior));
+
+    const next = (action) => {
+      expect(action).toBe(actionWithoutWorker);
+      setTimeout(() => {
+        expect(mockWorkerBehavior).toHaveBeenCalledTimes(0);
+        done();
+      }, 10);
+    };
+
+    middleware({ dispatch })(next)(actionWithoutWorker);
+  });
+
+  it('when the action needs a worker, it should pass along the action with ' +
+     '`next` so that the next middleware in the chain sees it', (done) => {
+    const middleware = createWorkerMiddleware(new Worker());
+
+    const next = (action) => {
+      expect(action).toBe(actionWithWorker);
+      setTimeout(() => {
+        expect(dispatch).toHaveBeenCalledWith(actionWithWorker);
+        done();
+      }, 10);
+    };
+
+    middleware({ dispatch })(next)(actionWithWorker);
+  });
+
+  it('when the action doesn\'t need a worker, it should also pass along the action with ' +
+     '`next` so that the next middleware in the chain sees it', (done) => {
+    const middleware = createWorkerMiddleware(new Worker());
+
+    const next = (action) => {
+      expect(action).toBe(actionWithoutWorker);
+      setTimeout(() => {
+        expect(dispatch).toHaveBeenCalledTimes(0);
+        done();
+      }, 10);
+    };
+
+    middleware({ dispatch })(next)(actionWithoutWorker);
+  });
+
+  it('when the action needs a worker, it should still pass along the action with ' +
+     '`next`, and it should `dispatch` the action returned from the worker', (done) => {
+    const actionFromWorker = {
+      type: 'WORKER_RETURN',
+      payload: {
+        data: 100,
+      },
+    };
+
+    const mockWorkerBehavior = jest.fn(() => actionFromWorker);
+    const middleware = createWorkerMiddleware(new Worker(mockWorkerBehavior));
+
+    const next = (action) => {
+      expect(action).toBe(actionWithWorker);
+      setTimeout(() => {
+        expect(mockWorkerBehavior).toHaveBeenCalledTimes(1);
+        expect(mockWorkerBehavior).toHaveBeenCalledWith(actionWithWorker);
+
+        expect(dispatch).toHaveBeenCalledTimes(1);
+        expect(dispatch).toHaveBeenCalledWith(actionFromWorker);
+        done();
+      }, 10);
+    };
+
+    middleware({ dispatch })(next)(actionWithWorker);
+  });
+});

src/index.js

@@ -8,26 +8,16 @@ const createWorkerMiddleware = (worker) => {
   */
 
   if (!worker) {
-    console.error( // eslint-disable-line no-console
-      'Fatal: `worker` is falsy.'
+    throw new Error(
+      `\`createWorkerMiddleware\` expects a worker instance as the argument. Instead received: ${worker}`,
     );
   } else if (!worker.postMessage) {
-    console.error( // eslint-disable-line no-console
-      'Fatal: `worker` doesn\'t have a `postMessage` method.'
+    throw new Error(
+      'The worker instance is expected to have a `postMessage` method.',
     );
   }
 
-  /*
-    the first argument is ({ dispatch, getState }) by default,
-    but we don't actually need them for now.
-  */
-  return ({ dispatch }) => (next) => {
-    if (!next) {
-      console.error( // eslint-disable-line no-console
-        'Fatal: worker middleware received no `next` action. Check your chain of middlewares.'
-      );
-    }
-
+  return ({ dispatch }) => {
     /*
       when the worker posts a message back, dispatch the action with its payload
       so that it will go through the entire middleware chain
@@ -36,12 +26,20 @@ const createWorkerMiddleware = (worker) => {
       dispatch(resultAction);
     };
 
-    return (action) => {
-      if (action.meta && action.meta.WebWorker) {
-        worker.postMessage(action);
+    return (next) => {
+      if (!next) {
+        throw new Error(
+          'Worker middleware received no `next` action. Check your chain of middlewares.',
+        );
       }
-      // always pass the action along to the next middleware
-      return next(action);
+
+      return (action) => {
+        if (action.meta && action.meta.WebWorker) {
+          worker.postMessage(action);
+        }
+        // always pass the action along to the next middleware
+        return next(action);
+      };
     };
   };
 };