Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($interpolate): use custom toString() function & fix(ngBind): use same json conversion as $interpolate #14715

Merged
merged 3 commits into from
Jun 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/content/guide/interpolation.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ normal {@link ng.$rootScope.Scope#$digest digest} cycle.

Note that the interpolateDirective has a priority of 100 and sets up the watch in the preLink function.

### How the string representation is computed

If the interpolated value is not a `String`, it is computed as follows:
- `undefined` and `null` are converted to `''`
- if the value is an object that is not a `Number`, `Date` or `Array`, $interpolate looks for
a custom `toString()` function on the object, and uses that. Custom means that
`myObject.toString !== `Object.prototype.toString`.
- if the above doesn't apply, `JSON.stringify` is used.

### Binding to boolean attributes

Attributes such as `disabled` are called `boolean` attributes, because their presence means `true` and
Expand Down
1 change: 1 addition & 0 deletions src/.jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
"createMap": false,
"VALIDITY_STATE_PROPERTY": false,
"reloadWithDebugInfo": false,
"stringify": false,

"NODE_TYPE_ELEMENT": false,
"NODE_TYPE_ATTRIBUTE": false,
Expand Down
22 changes: 22 additions & 0 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
getBlockNodes: true,
hasOwnProperty: true,
createMap: true,
stringify: true,

NODE_TYPE_ELEMENT: true,
NODE_TYPE_ATTRIBUTE: true,
Expand Down Expand Up @@ -1903,6 +1904,27 @@ function createMap() {
return Object.create(null);
}

function stringify(value) {
if (value == null) { // null || undefined
return '';
}
switch (typeof value) {
case 'string':
break;
case 'number':
value = '' + value;
break;
default:
if (hasCustomToString(value) && !isArray(value) && !isDate(value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I am a big fan of not toStringifying Dates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were jsonified before and that output is more universal than toString, which produces an American date format while jsonified is ISO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were JSONified in $interpolate, but toStringified in ngBind. Since we are making the two consistent, we need to break one of them.

But you convicned me: a universal format is better than forcing American English for all users.
(For some reason, I was under the impression toString() returned a localized string...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that too (localized string) and might have written that somewhere.

value = value.toString();
} else {
value = toJson(value);
}
}

return value;
}

var NODE_TYPE_ELEMENT = 1;
var NODE_TYPE_ATTRIBUTE = 2;
var NODE_TYPE_TEXT = 3;
Expand Down
3 changes: 2 additions & 1 deletion src/AngularPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ function publishExternalAPI(angular) {
'$$minErr': minErr,
'$$csp': csp,
'$$encodeUriSegment': encodeUriSegment,
'$$encodeUriQuery': encodeUriQuery
'$$encodeUriQuery': encodeUriQuery,
'$$stringify': stringify
});

angularModule = setupModuleLoader(window);
Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/ngBind.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ var ngBindDirective = ['$compile', function($compile) {
$compile.$$addBindingInfo(element, attr.ngBind);
element = element[0];
scope.$watch(attr.ngBind, function ngBindWatchAction(value) {
element.textContent = isUndefined(value) ? '' : value;
element.textContent = stringify(value);
});
};
}
Expand Down
17 changes: 0 additions & 17 deletions src/ng/interpolate.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,23 +111,6 @@ function $InterpolateProvider() {
replace(escapedEndRegexp, endSymbol);
}

function stringify(value) {
if (value == null) { // null || undefined
return '';
}
switch (typeof value) {
case 'string':
break;
case 'number':
value = '' + value;
break;
default:
value = toJson(value);
}

return value;
}

//TODO: this is the same as the constantWatchDelegate in parse.js
function constantWatchDelegate(scope, listener, objectEquality, constantInterp) {
var unwatch;
Expand Down
10 changes: 1 addition & 9 deletions src/ngMessageFormat/messageFormatCommon.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,7 @@
/* global isFunction: false */
/* global noop: false */
/* global toJson: false */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not necessary any more (but it may have gone away in master anyway).


function stringify(value) {
if (value == null /* null/undefined */) { return ''; }
switch (typeof value) {
case 'string': return value;
case 'number': return '' + value;
default: return toJson(value);
}
}
/* global $$stringify: false */

// Convert an index into the string into line/column for use in error messages
// As such, this doesn't have to be efficient.
Expand Down
5 changes: 3 additions & 2 deletions src/ngMessageFormat/messageFormatService.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
/* global noop: true */
/* global toJson: true */
/* global MessageFormatParser: false */
/* global stringify: false */

/**
* @ngdoc module
Expand Down Expand Up @@ -180,7 +179,7 @@ var $$MessageFormatFactory = ['$parse', '$locale', '$sce', '$exceptionHandler',
return function stringifier(value) {
try {
value = trustedContext ? $sce['getTrusted'](trustedContext, value) : $sce['valueOf'](value);
return allOrNothing && (value === void 0) ? value : stringify(value);
return allOrNothing && (value === void 0) ? value : $$stringify(value);
} catch (err) {
$exceptionHandler($interpolateMinErr['interr'](text, err));
}
Expand Down Expand Up @@ -214,6 +213,7 @@ var $interpolateMinErr;
var isFunction;
var noop;
var toJson;
var $$stringify;

var module = window['angular']['module']('ngMessageFormat', ['ng']);
module['factory']('$$messageFormat', $$MessageFormatFactory);
Expand All @@ -222,6 +222,7 @@ module['config'](['$provide', function($provide) {
isFunction = window['angular']['isFunction'];
noop = window['angular']['noop'];
toJson = window['angular']['toJson'];
$$stringify = window['angular']['$$stringify'];

$provide['decorator']('$interpolate', $$interpolateDecorator);
}]);
58 changes: 58 additions & 0 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1857,6 +1857,16 @@ describe('input', function() {
it('should parse ISO-based date strings as a valid min date value', function() {
var inputElm = helper.compileInput('<input name="myControl" type="date" min="{{ min }}" ng-model="value">');

$rootScope.value = new Date(2010, 1, 1, 0, 0, 0);
$rootScope.min = new Date(2014, 10, 10, 0, 0, 0).toISOString();
$rootScope.$digest();

expect($rootScope.form.myControl.$error.min).toBeTruthy();
});

it('should parse interpolated Date objects as a valid min date value', function() {
var inputElm = helper.compileInput('<input name="myControl" type="date" min="{{ min }}" ng-model="value">');

$rootScope.value = new Date(2010, 1, 1, 0, 0, 0);
$rootScope.min = new Date(2014, 10, 10, 0, 0, 0);
$rootScope.$digest();
Expand Down Expand Up @@ -1896,6 +1906,16 @@ describe('input', function() {
it('should parse ISO-based date strings as a valid max date value', function() {
var inputElm = helper.compileInput('<input name="myControl" type="date" max="{{ max }}" ng-model="value">');

$rootScope.value = new Date(2020, 1, 1, 0, 0, 0);
$rootScope.max = new Date(2014, 10, 10, 0, 0, 0).toISOString();
$rootScope.$digest();

expect($rootScope.form.myControl.$error.max).toBeTruthy();
});

it('should parse interpolated Date objects as a valid max date value', function() {
var inputElm = helper.compileInput('<input name="myControl" type="date" max="{{ max }}" ng-model="value">');

$rootScope.value = new Date(2020, 1, 1, 0, 0, 0);
$rootScope.max = new Date(2014, 10, 10, 0, 0, 0);
$rootScope.$digest();
Expand Down Expand Up @@ -1990,6 +2010,44 @@ describe('input', function() {
expect(inputElm).toBeValid();
});


it('should allow Date objects as valid ng-max values', function() {
$rootScope.max = new Date(2012, 1, 1, 1, 2, 0);
var inputElm = helper.compileInput('<input type="datetime-local" ng-model="value" name="alias" ng-max="max" />');

helper.changeInputValueTo('2014-01-01T12:34:00');
expect(inputElm).toBeInvalid();

$rootScope.max = new Date(2013, 1, 1, 1, 2, 0);
$rootScope.$digest();

expect(inputElm).toBeInvalid();

$rootScope.max = new Date(2014, 1, 1, 1, 2, 0);
$rootScope.$digest();

expect(inputElm).toBeValid();
});


it('should allow Date objects as valid ng-min values', function() {
$rootScope.min = new Date(2013, 1, 1, 1, 2, 0);
var inputElm = helper.compileInput('<input type="datetime-local" ng-model="value" name="alias" ng-min="min" />');

helper.changeInputValueTo('2010-01-01T12:34:00');
expect(inputElm).toBeInvalid();

$rootScope.min = new Date(2014, 1, 1, 1, 2, 0);
$rootScope.$digest();

expect(inputElm).toBeInvalid();

$rootScope.min = new Date(2009, 1, 1, 1, 2, 0);
$rootScope.$digest();

expect(inputElm).toBeValid();
});

describe('ISO_DATE_REGEXP', function() {
var dates = [
// Validate date
Expand Down
37 changes: 37 additions & 0 deletions test/ng/directive/ngBindSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,43 @@ describe('ngBind*', function() {
expect(element.text()).toEqual('-0false');
}));

they('should jsonify $prop', [[{a: 1}, '{"a":1}'], [true, 'true'], [false, 'false']], function(prop) {
inject(function($rootScope, $compile) {
$rootScope.value = prop[0];
element = $compile('<div ng-bind="value"></div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual(prop[1]);
});
});

Copy link
Member

@gkalpak gkalpak Jun 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a test with a custom toString here too.

it('should use custom toString when present', inject(function($rootScope, $compile) {
$rootScope.value = {
toString: function() {
return 'foo';
}
};
element = $compile('<div ng-bind="value"></div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual('foo');
}));

it('should NOT use toString on array objects', inject(function($rootScope, $compile) {
$rootScope.value = [];
element = $compile('<div ng-bind="value"></div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toEqual('[]');
}));


it('should NOT use toString on Date objects', inject(function($rootScope, $compile) {
$rootScope.value = new Date(2014, 10, 10, 0, 0, 0);
element = $compile('<div ng-bind="value"></div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toBe(JSON.stringify($rootScope.value));
expect(element.text()).not.toEqual($rootScope.value.toString());
}));


it('should one-time bind if the expression starts with two colons', inject(function($rootScope, $compile) {
element = $compile('<div ng-bind="::a"></div>')($rootScope);
$rootScope.a = 'lucas';
Expand Down
23 changes: 23 additions & 0 deletions test/ng/interpolateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,29 @@ describe('$interpolate', function() {
expect($interpolate('{{ false }}')({})).toEqual('false');
}));

it('should use custom toString when present', inject(function($interpolate, $rootScope) {
var context = {
a: {
toString: function() {
return 'foo';
}
}
};

expect($interpolate('{{ a }}')(context)).toEqual('foo');
}));

it('should NOT use toString on array objects', inject(function($interpolate) {
expect($interpolate('{{a}}')({ a: [] })).toEqual('[]');
}));


it('should NOT use toString on Date objects', inject(function($interpolate) {
var date = new Date(2014, 10, 10);
expect($interpolate('{{a}}')({ a: date })).toBe(JSON.stringify(date));
expect($interpolate('{{a}}')({ a: date })).not.toEqual(date.toString());
}));


it('should return interpolation function', inject(function($interpolate, $rootScope) {
var interpolateFn = $interpolate('Hello {{name}}!');
Expand Down
24 changes: 24 additions & 0 deletions test/ngMessageFormat/messageFormatSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,30 @@ describe('$$ngMessageFormat', function() {
}));


it('should use custom toString when present', inject(function($interpolate, $rootScope) {
var context = {
a: {
toString: function() {
return 'foo';
}
}
};

expect($interpolate('{{ a }}')(context)).toEqual('foo');
}));

it('should NOT use toString on array objects', inject(function($interpolate) {
expect($interpolate('{{a}}')({ a: [] })).toEqual('[]');
}));


it('should NOT use toString on Date objects', inject(function($interpolate) {
var date = new Date(2014, 10, 10);
expect($interpolate('{{a}}')({ a: date })).toBe(JSON.stringify(date));
expect($interpolate('{{a}}')({ a: date })).not.toEqual(date.toString());
}));


it('should return interpolation function', inject(function($interpolate, $rootScope) {
var interpolateFn = $interpolate('Hello {{name}}!');

Expand Down