treating context as an argument. closes #3

.flowconfig

@@ -5,3 +5,7 @@
 [libs]
 
 [options]
+
+# Provides a way to suppress flow errors in the following line.
+# Example: // $FlowSuppressError: This following line is borked because of reasons.
+suppress_comment= \\(.\\|\n\\)*\\$FlowSuppressError

README.md

@@ -85,11 +85,11 @@ yarn add memoize-one
 npm install memoize-one --save
 ```
 
-## Other
+## Correctly handling execution context (`this`)
 
 ### memoizeOne correctly supports `this` control
 
-This library takes special care to maintain, and allow control over the the `this` context just like a regular function. Both the original function and the memoized function's can be controlled using all the standard `this` controlling techniques:
+This library takes special care to maintain, and allow control over the the `this` context for **both** the original function being memoized as well as the returned memoized function. Both the original function and the memoized function's `this` context respect all the standard `this` controlling techniques:
 
 - new bindings (`new`)
 - explicit binding (`call`, `apply`, `bind`);
@@ -98,9 +98,35 @@ This library takes special care to maintain, and allow control over the the `thi
 - fat arrow binding (binding to lexical `this`)
 - ignored this (pass `null` as `this` to explicit binding)
 
-### Code health
+### Changes to `this` are considered argument changes
+
+Changes to the running context (`this`) of a function can result in the function returning a different value event though its arguments have stayed the same:
+
+```js
+function getA() {
+    return this.a;
+}
+
+const temp1 = {
+    a: 20,
+};
+const temp2 = {
+    a: 30,
+}
+
+getA.call(temp1); // 20
+getA.call(temp2); // 30
+```
+
+Therefore, in order to prevent against unexpected results, `memoizeOne` takes into account the current execution context (`this`) of the memoized function. If `this` is different to the previous invokation then it is considered a change in argument. [further discussion](https://github.com/alexreardon/memoize-one/issues/3).
+
+Generally this will be of no impact if you are not explicity controlling the `this` context of functions you want to memoize with [explicit binding](https://github.com/getify/You-Dont-Know-JS/blob/master/this%20%26%20object%20prototypes/ch2.md#explicit-binding)  or [implicit binding](https://github.com/getify/You-Dont-Know-JS/blob/master/this%20%26%20object%20prototypes/ch2.md#implicit-binding). `memoizeOne` will detect when you are manipulating `this` and will then consider the `this` context as an argument. If `this` changes, it will re-execute the original function even if the arguments have not changed.
+
+
+## Code health
 
 - Tested with all built in [JavaScript types](https://github.com/getify/You-Dont-Know-JS/blob/master/types%20%26%20grammar/ch1.md).
 - 100% code coverage.
 - [flow types](http://flowtype.org) for safer internal execution and external consumption. Also allows for editor autocompletion.
-- Follows [Semantic versioning (2.0)](http://semver.org/) for safer versioning.
+- Follows [Semantic versioning (2.0)](http://semver.org/) for safer versioning.
+- Lightweight with no dependencies

package.json

@@ -1,7 +1,7 @@
 {
   "name": "memoize-one",
-  "version": "1.0.0-rc.2",
-  "description": "A memoization library for memoizing a function with a cache size of one",
+  "version": "1.0.0-rc.3",
+  "description": "A memoization library which only remembers the latest invokation",
   "main": "lib/index.js",
   "module": "src/index.js",
   "author": "Alex Reardon <alexreardon@gmail.com>",
@@ -44,5 +44,10 @@
     "coverage:report": "nyc report --reporter=text-lcov > coverage.lcov",
     "coverage:publish": "codecov",
     "prepublish": "yarn run build"
-  }
+  },
+  "keywords": [
+    "memoize",
+    "cache",
+    "performance"
+  ]
 }

src/index.js

@@ -9,14 +9,17 @@ export default function (resultFn: Function, isEqual?: EqualityFn = simpleIsEqua
     let lastResult: any;
     let calledOnce: boolean = false;
 
+    // breaking cache when arguments or context changes
     return function (...newArgs: Array<any>) {
         if (calledOnce &&
+            lastThis === this &&
             newArgs.length === lastArgs.length &&
             lastArgs.every((lastArg, i) => isEqual(lastArg, newArgs[i]))) {
             return lastResult;
         }
 
         calledOnce = true;
+        lastThis = this;
         lastArgs = newArgs;
         lastResult = resultFn.apply(this, newArgs);
         return lastResult;

test/index.js

@@ -4,18 +4,12 @@ import { expect } from 'chai';
 import sinon from 'sinon';
 import memoizeOne from '../src/';
 
-type Expectation = {|
-    args: any[],
-        result: any
-            |};
-
-type Input = {|
-    name: string,
-        first: Expectation,
-            second: Expectation
-                |};
-
 describe('memoizeOne', () => {
+    function getA() {
+        // $FlowSuppressError: allowing many values for `this`
+        return this.a;
+    }
+
     describe('standard behaviour - baseline', () => {
         let add;
         let memoizedAdd;
@@ -57,7 +51,18 @@ describe('memoizeOne', () => {
         });
     });
 
-    describe('standard behaviour', () => {
+    describe('standard behaviour - dynamic', () => {
+        type Expectation = {|
+            args: any[],
+            result: any
+        |};
+
+        type Input = {|
+            name: string,
+            first: Expectation,
+            second: Expectation
+        |};
+
         // [JavaScript defines seven built-in types:](https://github.com/getify/You-Dont-Know-JS/blob/master/types%20%26%20grammar/ch1.md)
         //    - null
         //    - undefined
@@ -168,7 +173,7 @@ describe('memoizeOne', () => {
         };
 
         inputs.forEach(({ name, first, second }) => {
-            describe(`type test:[${name}]`, () => {
+            describe(`type: [${name}]`, () => {
 
                 let spy;
                 let memoized;
@@ -224,10 +229,6 @@ describe('memoizeOne', () => {
 
     describe('respecting "this" context', () => {
         describe('original function', () => {
-            function getA() {
-                return this.a;
-            }
-
             it('should respect new bindings', () => {
                 const Foo = function (bar) {
                     this.bar = bar;
@@ -285,7 +286,7 @@ describe('memoizeOne', () => {
                     // return an arrow function
                     return () => {
                         // `this` here is lexically adopted from `foo()`
-                        return this.a;
+                        return getA.call(this);
                     };
                 }
                 const bound = foo.call(temp);
@@ -303,11 +304,19 @@ describe('memoizeOne', () => {
         });
 
         describe('memoized function', () => {
-            function getA() {
-                return this.a;
-            }
+            it('should respect new bindings', () => {
+                const memoizedGetA = memoizeOne(getA);
+                const Foo = function (a) {
+                    this.a = a;
+                    this.result = memoizedGetA.call(this);
+                };
 
-            it('should respect new bindings');
+                const foo1 = new Foo(10);
+                const foo2 = new Foo(20);
+
+                expect(foo1.result).to.equal(10);
+                expect(foo2.result).to.equal(20);
+            });
 
             it('should respect implicit bindings', () => {
                 const getAMemoized = memoizeOne(getA);
@@ -363,7 +372,21 @@ describe('memoizeOne', () => {
             });
 
             it('should respect fat arrow bindings', () => {
+                const temp = {
+                    a: 50,
+                };
+                const memoizedGetA = memoizeOne(getA);
+                function foo() {
+                    // return an arrow function
+                    return () => {
+                        // `this` here is lexically adopted from `foo()`
+                        return memoizedGetA.call(this);
+                    };
+                }
+                const bound = foo.call(temp);
+                const memoized = memoizeOne(bound);
 
+                expect(memoized()).to.equal(50);
             });
 
             it('should respect ignored bindings', () => {
@@ -375,25 +398,23 @@ describe('memoizeOne', () => {
 
                 expect(getResult).to.throw(TypeError);
             });
+        });
+    });
 
-            // This behaviour is debatable.
-            // For: sometimes you might call a function in different `this` without knowing it and this would bust your cache
-            // Against: if your function is reading a value from `this` then your memoized function would be returning the wrong result.
-            it('should memoize the previous result even if the this context changes', () => {
-                const memoized = memoizeOne(getA);
-                const temp1 = {
-                    a: 20,
-                    getMemoizedA: memoized,
-                };
-                const temp2 = {
-                    a: 30,
-                    getMemoizedA: memoized,
-                };
-
-                expect(temp1.getMemoizedA()).to.equal(20);
-                // this might be unexpected
-                expect(temp2.getMemoizedA()).to.equal(20);
-            });
+    describe('context change', () => {
+        it('should break the memoization cache if the execution context changes', () => {
+            const memoized = memoizeOne(getA);
+            const temp1 = {
+                a: 20,
+                getMemoizedA: memoized,
+            };
+            const temp2 = {
+                a: 30,
+                getMemoizedA: memoized,
+            };
+
+            expect(temp1.getMemoizedA()).to.equal(20);
+            expect(temp2.getMemoizedA()).to.equal(30);
         });
     });