fix: remove cache for different options (#5)

patrickhulce · Feb 27, 2018 · 058d709 · 058d709
1 parent 5ce55fe
commit 058d709
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 8 deletions.
diff --git a/packages/klay-kiln/lib/kiln.js b/packages/klay-kiln/lib/kiln.js
@@ -42,22 +42,22 @@ module.exports = function () {
 
   function bake(modelName, extension, options) {
     var modelDef = getModelDefOrThrow(modelName);
-    if (typeof extension === 'string' && typeof options === 'undefined') {
+    if (typeof extension === 'string') {
       var extensionName = extension;
       var extensions = getExtensions(modelDef);
       var extensionDef = _.find(extensions, item => item.extension.name === extensionName);
       assert.ok(extensionDef, `could not find extension: ${extensionName}`);
       extension = _.get(extensionDef, 'extension');
-      options = _.get(extensionDef, 'options');
+      options = _.assign({}, _.get(extensionDef, 'options'), options);
     }
 
     options = options || {};
     validateExtension(extension);
 
     // Check if we've already baked this model
     var prebakedInst = _.get(prebaked, [modelName, extension.name]);
-    if (prebakedInst) {
-      return prebakedInst;
+    if (prebakedInst && _.isEqual(prebakedInst.options, options)) {
+      return prebakedInst.inst;
     }
 
     var determineDeps = _.get(extension, 'determineDependencies', _.noop);
@@ -66,7 +66,7 @@ module.exports = function () {
     var result = extension.bake(modelDef, options, dependencies);
 
     var prebakedOfModel = prebaked[modelName] || {};
-    prebakedOfModel[extension.name] = result;
+    prebakedOfModel[extension.name] = {inst: result, options};
     prebaked[modelName] = prebakedOfModel;
 
     return result;

diff --git a/packages/klay-kiln/test/kiln.test.js b/packages/klay-kiln/test/kiln.test.js
@@ -181,21 +181,21 @@ defineTest('kiln.js', function (Kiln) {
       dep1Args.should.have.deep.property('0.name', 'user');
       dep1Args.should.have.deep.property('0.model', userModel);
       dep1Args.should.have.deep.property('0.metadata', userOptions);
-      dep1Args.should.have.property('1', dep1Options);
+      dep1Args.should.have.property('1').eql(dep1Options);
       dep1Args.should.have.property('2').eql({});
 
       var dep2Args = dep2Stub.firstCall.args;
       dep2Args.should.have.deep.property('0.name', 'photo');
       dep2Args.should.have.deep.property('0.model', photoModel);
       dep2Args.should.have.deep.property('0.metadata').eql({});
-      dep2Args.should.have.property('1', dep2Options);
+      dep2Args.should.have.property('1').eql(dep2Options);
       dep2Args.should.have.property('2').eql({});
 
       var bakeArgs = bakeStub.firstCall.args;
       bakeArgs.should.have.deep.property('0.name', 'user');
       bakeArgs.should.have.deep.property('0.model', userModel);
       bakeArgs.should.have.deep.property('0.metadata', userOptions);
-      bakeArgs.should.have.property('1', extensionOptions);
+      bakeArgs.should.have.property('1').eql(extensionOptions);
       bakeArgs.should.have.property('2').eql({dep1: {result: 'dep1'}, 'photo:dep2': {result: 'dep2'}});
     });
 
@@ -207,6 +207,16 @@ defineTest('kiln.js', function (Kiln) {
       extensionABake.should.have.been.calledOnce;
     });
 
+    it('should not cache results of already baked extensions with different options', function () {
+      complexKiln();
+      var resultA = kiln.bake('user', 'A');
+      var resultB = kiln.bake('user', 'A', {different: 'option'});
+      var resultC = kiln.bake('user', 'A');
+      resultA.should.equal(resultB);
+      resultB.should.equal(resultC);
+      extensionABake.should.have.been.calledThrice;
+    });
+
     it('should fail when referencing an unknown model', function () {
       (function () {
         complexKiln();