Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Commit

Permalink
fix(compiler): OneWayOneTime bindings now wait for the model to stablize
Browse files Browse the repository at this point in the history
BREAKING CHANGE

Previously, OneWayOneTime bindings would remove their watch after the
first value assignment.  For models that take more than one digest loop
to stablize, this meant the OneWayOneTime bindings took a un-stablized
value.

With this change, these bindings will continue to accept value
assignments until their stablized value is non-null. The assignment may
occur multiple times.

We expect that most uses of OneWayOneTime will be 'ok' with
being called a few times as the model stablizes.

However, applications which depend on OneWayOneTime bindings being
called exactly once will break.  The suggested upgrade path is to
move the senstive code into a domWrite callback. e.g. the old code:

    @decorator(
        map: const { 'one-time': '=>!value' })
    class OneTime {
      set value(v) => onlyCalledOnce(v);
    }

becomes

    @decorator(
        map: const { 'one-time': '=>!value' })
    class OneTime {
      var _value;
      set value(v) {
        if (_value == null) {
	  scope.rootScope.domWrite(() => onlyCalledOnce(_value));
	}
      	_value = v;
      }
    }

Closes #1013
  • Loading branch information
jbdeboer committed May 13, 2014
1 parent 5ad9c7a commit 0e12949
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 3 deletions.
13 changes: 11 additions & 2 deletions lib/core_dom/element_binder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,18 @@ class ElementBinder {

Expression attrExprFn = _parser(nodeAttrs[attrName]);
var watch;
var lastOneTimeValue;
watch = scope.watch(nodeAttrs[attrName], (value, _) {
if (dstPathFn.assign(controller, value) != null) {
watch.remove();
if ((lastOneTimeValue = dstPathFn.assign(controller, value)) != null && watch != null) {
var watchToRemove = watch;
watch = null;
scope.rootScope.domWrite(() {
if (lastOneTimeValue != null) {
watchToRemove.remove();
} else { // It was set to non-null, but stablized to null, wait.
watch = watchToRemove;
}
});
}
}, formatters: formatters);
break;
Expand Down
72 changes: 71 additions & 1 deletion test/core_dom/compiler_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ void main() {
..bind(SometimesComponent)
..bind(ExprAttrComponent)
..bind(LogElementComponent)
..bind(SayHelloFormatter);
..bind(SayHelloFormatter)
..bind(OneTimeDecorator);
});

it('should select on element', async(() {
Expand Down Expand Up @@ -690,6 +691,64 @@ void main() {
}
}));
});

describe('bindings', () {
it('should set a one-time binding with the correct value', (Logger logger) {
_.compile(r'<div one-time="v"></div>');

_.rootScope.context['v'] = 1;

var context = _.rootScope.context;
_.rootScope.watch('3+4', (v, _) => context['v'] = v);

// In the 1st digest iteration:
// v will be set to 7
// OneTimeDecorator.value will be set to 1
// In the 2nd digest iteration:
// OneTimeDecorator.value will be set to 7
_.rootScope.apply();

expect(logger).toEqual([1, 7]);
});

it('should keep one-time binding until it is set to non-null', (Logger logger) {
_.compile(r'<div one-time="v"></div>');
_.rootScope.context['v'] = null;
_.rootScope.apply();
expect(logger).toEqual([null]);

_.rootScope.context['v'] = 7;
_.rootScope.apply();
expect(logger).toEqual([null, 7]);

// Check that the binding is removed.
_.rootScope.context['v'] = 8;
_.rootScope.apply();
expect(logger).toEqual([null, 7]);
});

it('should remove the one-time binding only if it stablizied to null', (Logger logger) {
_.compile(r'<div one-time="v"></div>');

_.rootScope.context['v'] = 1;

var context = _.rootScope.context;
_.rootScope.watch('3+4', (v, _) => context['v'] = null);

_.rootScope.apply();
expect(logger).toEqual([1, null]);

// Even though there was a null in the unstable model, we shouldn't remove the binding
context['v'] = 8;
_.rootScope.apply();
expect(logger).toEqual([1, null, 8]);

// Check that the binding is removed.
_.rootScope.context['v'] = 9;
_.rootScope.apply();
expect(logger).toEqual([1, null, 8]);
});
});
});


Expand Down Expand Up @@ -1130,3 +1189,14 @@ class LogElementComponent{
logger(shadowRoot);
}
}

@Decorator(
selector: '[one-time]',
map: const {
'one-time': '=>!value'
})
class OneTimeDecorator {
Logger log;
OneTimeDecorator(this.log);
set value(v) => log(v);
}

0 comments on commit 0e12949

Please sign in to comment.