Skip to content

Commit

Permalink
[BUGFIX beta] {{get}} helper subscribes to values and can be updated
Browse files Browse the repository at this point in the history
This commit fixes two issues:

1. When using the {{get}} helper it sometimes wouldn't update correctly.

Because a helper and a keyword are being used, ember is invoking the helper logic and wrapping the stream the get keyword creates in a BuiltInHelperStream - it also would not subscribe the morph to the stream. This subscription is where the error lies.

To fix this the helper has been removed and only keyword logic is implemented.

2. It was previously not possible to use the {{input}} helper with the {{get}} helper - this functionality has now been added using the mut keyword, e.g:

{{input value=(mut (get source path)) type='text'}}
  • Loading branch information
jmurphyau committed Jul 16, 2015
1 parent 008bcd9 commit de5446f
Show file tree
Hide file tree
Showing 4 changed files with 380 additions and 258 deletions.
14 changes: 0 additions & 14 deletions packages/ember-htmlbars/lib/helpers/-get.js

This file was deleted.

133 changes: 93 additions & 40 deletions packages/ember-htmlbars/lib/keywords/get.js
Original file line number Diff line number Diff line change
@@ -1,72 +1,125 @@
import Ember from 'ember-metal/core';
import isEnabled from 'ember-metal/features';
import Stream from 'ember-metal/streams/stream';
import { labelFor } from 'ember-metal/streams/utils';
import { read, isStream } from 'ember-metal/streams/utils';
import KeyStream from 'ember-metal/streams/key-stream';
import { isStream } from 'ember-metal/streams/utils';
import merge from 'ember-metal/merge';
import subscribe from 'ember-htmlbars/utils/subscribe';
import { get } from 'ember-metal/property_get';
import { set } from 'ember-metal/property_set';
import {
addObserver,
removeObserver
} from 'ember-metal/observer';

function labelFor(source, key) {
const sourceLabel = source.label ? source.label : '';
const keyLabel = key.label ? key.label : '';
return `(get ${sourceLabel} ${keyLabel})`;
}

if (isEnabled('ember-htmlbars-get-helper')) {
var getKeyword = function getKeyword(morph, env, scope, params, hash, template, inverse, visitor) {
var objParam = params[0];
var pathParam = params[1];
const buildStream = function buildStream(params) {
const [objRef, pathRef] = params;

Ember.assert('The first argument to {{get}} must be a stream', isStream(objParam));
Ember.assert('The first argument to {{get}} must be a stream', isStream(objRef));
Ember.assert('{{get}} requires at least two arguments', params.length > 1);

var getStream = new GetStream(objParam, pathParam);
const stream = new DynamicKeyStream(objRef, pathRef);

return stream;
};

if (morph === null) {
return getStream;
var getKeyword = function getKeyword(morph, env, scope, params, hash, template, inverse, visitor) {
if (!morph) {
return buildStream(params);
} else {
env.hooks.inline(morph, env, scope, '-get', [getStream], hash, visitor);
let stream;
if (morph.linkedResult) {
stream = morph.linkedResult;
} else {
stream = buildStream(params);

subscribe(morph, env, scope, stream);
env.hooks.linkRenderNode(morph, env, scope, null, params, hash);

morph.linkedResult = stream;
}
env.hooks.range(morph, env, scope, null, stream, visitor);
}

return true;
};

var GetStream = function GetStream(obj, path) {
this.init(`(get ${labelFor(obj)} ${labelFor(path)})`);

this.objectParam = obj;
this.pathParam = path;
this.lastPathValue = undefined;
this.valueDep = this.addMutableDependency();
var DynamicKeyStream = function DynamicKeyStream(source, keySource) {
if (!isStream(keySource)) {
return new KeyStream(source, keySource);
}
Ember.assert('DynamicKeyStream error: source must be a stream', isStream(source)); // TODO: This isn't necessary.

this.addDependency(path);
// used to get the original path for debugging and legacy purposes
var label = labelFor(source, keySource);

// This next line is currently only required when the keyword
// is executed in a subexpression. More investigation required
// to remove the additional dependency
this.addDependency(obj);
this.init(label);
this.path = label;
this.sourceDep = this.addMutableDependency(source);
this.keyDep = this.addMutableDependency(keySource);
this.observedObject = null;
this.observedKey = null;
};

GetStream.prototype = Object.create(Stream.prototype);

merge(GetStream.prototype, {
updateValueDependency() {
var pathValue = read(this.pathParam);

if (this.lastPathValue !== pathValue) {
if (typeof pathValue === 'string') {
this.valueDep.replace(this.objectParam.get(pathValue));
} else {
this.valueDep.replace();
}
DynamicKeyStream.prototype = Object.create(KeyStream.prototype);

this.lastPathValue = pathValue;
merge(DynamicKeyStream.prototype, {
key() {
const key = this.keyDep.getValue();
if (typeof key === 'string') {
Ember.assert('DynamicKeyStream error: key must not have a \'.\'', key.indexOf('.') === -1);
return key;
}
},

compute() {
this.updateValueDependency();
return this.valueDep.getValue();
var object = this.sourceDep.getValue();
var key = this.key();
if (object && key) {
return get(object, key);
}
},

setValue(value) {
this.updateValueDependency();
this.valueDep.setValue(value);
}
var object = this.sourceDep.getValue();
var key = this.key();
if (object) {
set(object, key, value);
}
},

_super$revalidate: Stream.prototype.revalidate,

revalidate(value) {
this._super$revalidate(value);

var object = this.sourceDep.getValue();
var key = this.key();
if (object !== this.observedObject || key !== this.observedKey) {
this._clearObservedObject();

if (object && typeof object === 'object' && key) {
addObserver(object, key, this, this.notify);
this.observedObject = object;
this.observedKey = key;
}
}
},

_clearObservedObject() {
if (this.observedObject) {
removeObserver(this.observedObject, this.observedKey, this, this.notify);
this.observedObject = null;
this.observedKey = null;
}
}
});
}

Expand Down
4 changes: 0 additions & 4 deletions packages/ember-htmlbars/lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import concatHelper from 'ember-htmlbars/helpers/-concat';
import joinClassesHelper from 'ember-htmlbars/helpers/-join-classes';
import legacyEachWithControllerHelper from 'ember-htmlbars/helpers/-legacy-each-with-controller';
import legacyEachWithKeywordHelper from 'ember-htmlbars/helpers/-legacy-each-with-keyword';
import getHelper from 'ember-htmlbars/helpers/-get';
import htmlSafeHelper from 'ember-htmlbars/helpers/-html-safe';
import DOMHelper from 'ember-htmlbars/system/dom-helper';
import Helper, { helper as makeHelper } from 'ember-htmlbars/helper';
Expand All @@ -79,9 +78,6 @@ registerHelper('concat', concatHelper);
registerHelper('-join-classes', joinClassesHelper);
registerHelper('-legacy-each-with-controller', legacyEachWithControllerHelper);
registerHelper('-legacy-each-with-keyword', legacyEachWithKeywordHelper);
if (isEnabled('ember-htmlbars-get-helper')) {
registerHelper('-get', getHelper);
}
registerHelper('-html-safe', htmlSafeHelper);

Ember.HTMLBars = {
Expand Down
Loading

0 comments on commit de5446f

Please sign in to comment.