Skip to content

fix(renderer): stop performing value conversions #806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2017
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
16 changes: 0 additions & 16 deletions nativescript-angular/common/utils.ts

This file was deleted.

3 changes: 1 addition & 2 deletions nativescript-angular/directives/tab-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
import { TabView, TabViewItem } from "tns-core-modules/ui/tab-view";

import { CommentNode } from "../element-types";
import { convertToInt } from "../common/utils";
import { rendererLog } from "../trace";
import { isBlank } from "../lang-facade";

Expand All @@ -28,7 +27,7 @@ export class TabViewDirective implements AfterViewInit {
}

set selectedIndex(value) {
this._selectedIndex = convertToInt(value);
this._selectedIndex = value;
if (this.viewInitialized) {
this.tabView.selectedIndex = this._selectedIndex;
}
Expand Down
4 changes: 0 additions & 4 deletions nativescript-angular/lang-facade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ export function isBlank(obj: any): boolean {
return obj === undefined || obj === null;
}

export function isNumber(obj: any): boolean {
return typeof obj === 'number';
}

export function isDate(obj: any): obj is Date {
return obj instanceof Date && !isNaN(obj.valueOf());
}
Expand Down
11 changes: 1 addition & 10 deletions nativescript-angular/value-accessors/checked-value-accessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Directive, ElementRef, forwardRef, HostListener } from "@angular/core";
import { NG_VALUE_ACCESSOR } from "@angular/forms";
import { isBlank } from "../lang-facade";
import { BaseValueAccessor } from "./base-value-accessor";
import { Switch } from "tns-core-modules/ui/switch";

Expand Down Expand Up @@ -34,15 +33,7 @@ export class CheckedValueAccessor extends BaseValueAccessor<Switch> { // tslint:
}

writeValue(value: any): void {
let normalizedValue = false;
if (!isBlank(value)) {
if (typeof value === "string") {
normalizedValue = value.toLowerCase() === "true" ? true : false;
} else {
normalizedValue = !!value;
}
}
this.view.checked = normalizedValue;
this.view.checked = value;
}

registerOnTouched(fn: () => void): void { this.onTouched = fn; }
Expand Down
15 changes: 1 addition & 14 deletions nativescript-angular/value-accessors/date-value-accessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Directive, ElementRef, forwardRef, HostListener } from "@angular/core";
import { NG_VALUE_ACCESSOR } from "@angular/forms";
import { isBlank, isDate } from "../lang-facade";
import { BaseValueAccessor } from "./base-value-accessor";
import { DatePicker } from "tns-core-modules/ui/date-picker";

Expand Down Expand Up @@ -34,19 +33,7 @@ export class DateValueAccessor extends BaseValueAccessor<DatePicker> { // tslint
}

writeValue(value: any): void {
let normalizedValue = isBlank(value) ? new Date() : value;
if (!isDate(normalizedValue)) {
if (typeof normalizedValue === "string") {
normalizedValue = new Date(normalizedValue);
} else if (typeof normalizedValue === "number") {
normalizedValue = new Date(normalizedValue);
}

if (!isDate(normalizedValue)) {
normalizedValue = new Date();
}
}
this.view.date = normalizedValue;
this.view.date = value;
}

registerOnTouched(fn: () => void): void { this.onTouched = fn; }
Expand Down
14 changes: 1 addition & 13 deletions nativescript-angular/value-accessors/number-value-accessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Directive, ElementRef, forwardRef, HostListener } from "@angular/core";
import { NG_VALUE_ACCESSOR } from "@angular/forms";
import { isBlank, isNumber } from "../lang-facade";
import { BaseValueAccessor } from "./base-value-accessor";
import { Slider } from "tns-core-modules/ui/slider";

Expand Down Expand Up @@ -34,18 +33,7 @@ export class NumberValueAccessor extends BaseValueAccessor<Slider> { // tslint:d
}

writeValue(value: any): void {
let normalizedValue;
if (isBlank(value)) {
normalizedValue = 0;
} else {
if (isNumber(value)) {
normalizedValue = value;
} else {
let parsedValue = Number(value);
normalizedValue = isNaN(parsedValue) ? 0 : parsedValue;
}
}
this.view.value = normalizedValue;
this.view.value = value;
}

registerOnTouched(fn: () => void): void { this.onTouched = fn; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Directive, ElementRef, forwardRef, AfterViewInit, HostListener } from "
import { NG_VALUE_ACCESSOR } from "@angular/forms";
import { BaseValueAccessor } from "./base-value-accessor";
import { View } from "tns-core-modules/ui/core/view";
import { convertToInt } from "../common/utils";

const SELECTED_INDEX_VALUE_ACCESSOR = {provide: NG_VALUE_ACCESSOR,
useExisting: forwardRef(() => SelectedIndexValueAccessor), multi: true};
Expand Down Expand Up @@ -35,19 +34,19 @@ export class SelectedIndexValueAccessor extends BaseValueAccessor<SelectableView
super(elementRef.nativeElement);
}

private _normalizedValue: number;
private value: number;
private viewInitialized: boolean;

writeValue(value: any): void {
this._normalizedValue = convertToInt(value);
this.value = value;
if (this.viewInitialized) {
this.view.selectedIndex = this._normalizedValue;
this.view.selectedIndex = this.value;
}
}

ngAfterViewInit() {
this.viewInitialized = true;
this.view.selectedIndex = this._normalizedValue;
this.view.selectedIndex = this.value;
}

registerOnTouched(fn: () => void): void { this.onTouched = fn; }
Expand Down
4 changes: 1 addition & 3 deletions nativescript-angular/value-accessors/text-value-accessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Directive, ElementRef, forwardRef, HostListener } from "@angular/core";
import { NG_VALUE_ACCESSOR } from "@angular/forms";
import { isBlank } from "../lang-facade";
import { BaseValueAccessor } from "./base-value-accessor";
import { View } from "tns-core-modules/ui/core/view";

Expand Down Expand Up @@ -36,8 +35,7 @@ export class TextValueAccessor extends BaseValueAccessor<TextView> { // tslint:d
}

writeValue(value: any): void {
let normalizedValue = isBlank(value) ? "" : value.toString();
this.view.text = normalizedValue;
this.view.text = value;
}

registerOnTouched(fn: () => void): void { this.onTouched = fn; }
Expand Down
14 changes: 1 addition & 13 deletions nativescript-angular/value-accessors/time-value-accessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Directive, ElementRef, forwardRef, HostListener } from "@angular/core";
import { NG_VALUE_ACCESSOR } from "@angular/forms";
import { isBlank, isDate } from "../lang-facade";
import { BaseValueAccessor } from "./base-value-accessor";
import { TimePicker } from "tns-core-modules/ui/time-picker";

Expand Down Expand Up @@ -34,18 +33,7 @@ export class TimeValueAccessor extends BaseValueAccessor<TimePicker> { // tslint
}

writeValue(value: any): void {
let normalizedValue = isBlank(value) ? new Date() : value;
if (!isDate(normalizedValue)) {
if (typeof normalizedValue === "string") {
normalizedValue = new Date(normalizedValue);
} else if (typeof normalizedValue === "number") {
normalizedValue = new Date(normalizedValue);
}
if (!isDate(normalizedValue)) {
normalizedValue = new Date();
}
}
this.view.time = normalizedValue;
this.view.time = value;
}

registerOnTouched(fn: () => void): void { this.onTouched = fn; }
Expand Down
40 changes: 16 additions & 24 deletions nativescript-angular/view-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,41 +207,32 @@ export class ViewUtil {


private setPropertyInternal(view: NgView, attributeName: string, value: any): void {
traceLog("Setting attribute: " + attributeName);

let propMap = this.getProperties(view);
traceLog(`Setting attribute: ${attributeName}=${value} to ${view}`);

if (attributeName === "class") {
this.setClasses(view, value);
} else if (XML_ATTRIBUTES.indexOf(attributeName) !== -1) {
view._applyXmlAttribute(attributeName, value);
} else if (propMap.has(attributeName)) {
// We have a lower-upper case mapped property.
let propertyName = propMap.get(attributeName);
view[propertyName] = this.convertValue(value);
} else {
// Unknown attribute value -- just set it to our object as is.
view[attributeName] = this.convertValue(value);
return;
}
}

private convertValue(value: any): any {
if (typeof (value) !== "string" || value === "") {
return value;
if (XML_ATTRIBUTES.indexOf(attributeName) !== -1) {
view._applyXmlAttribute(attributeName, value);
return;
}

let valueAsNumber = +value;
if (!isNaN(valueAsNumber)) {
return valueAsNumber;
} else if (value && (value.toLowerCase() === "true" || value.toLowerCase() === "false")) {
return value.toLowerCase() === "true" ? true : false;
} else {
return value;
const propMap = this.getProperties(view);
const propertyName = propMap.get(attributeName);
if (propertyName) {
// We have a lower-upper case mapped property.
view[propertyName] = value;
return;
}

// Unknown attribute value -- just set it to our object as is.
view[attributeName] = value;
}

private getProperties(instance: any): Map<string, string> {
let type = instance && instance.constructor;
const type = instance && instance.constructor;
if (!type) {
return new Map<string, string>();
}
Expand All @@ -253,6 +244,7 @@ export class ViewUtil {
}
propertyMaps.set(type, propMap);
}

return propertyMaps.get(type);
}

Expand Down
14 changes: 9 additions & 5 deletions tests/app/tests/property-sets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,24 @@ describe("setting View properties", () => {
assert.equal("blah", view.stringValue);
});

it("converts number values", () => {
it("doesn\'t convert number values", () => {
let view = new TestView();
viewUtil.setProperty(view, "numValue", "42");
assert.strictEqual(42, view.numValue);
assert.strictEqual("42", view.numValue);

viewUtil.setProperty(view, "numValue", "42.");
assert.strictEqual("42.", view.numValue);

viewUtil.setProperty(view, "numValue", 0);
assert.strictEqual(0, view.numValue);
});

it("converts boolean values", () => {
it("doesn\'t convert boolean values", () => {
let view = new TestView();
viewUtil.setProperty(view, "boolValue", "true");
assert.strictEqual(true, view.boolValue);
assert.strictEqual("true", view.boolValue);
viewUtil.setProperty(view, "boolValue", "false");
assert.strictEqual(false, view.boolValue);
assert.strictEqual("false", view.boolValue);
});

it("sets style values", () => {
Expand Down
32 changes: 11 additions & 21 deletions tests/app/tests/value-accessor-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,72 +78,62 @@ describe("two-way binding via ng-model", () => {
assert.strictEqual(42, accessor.view.value);

accessor.writeValue("blah");
assert.strictEqual(0, accessor.view.value, "default to 0 on parse errors");
assert.notEqual(accessor.view.value, accessor.view.value, "defaults to NaN on parse errors");
});

it("converts strings to bools", () => {
const accessor = new TestCheckedValueAccessor();

accessor.writeValue(null);
assert.strictEqual(false, accessor.view.checked, "default to false on empty");
assert.strictEqual(null, accessor.view.checked, "default to null on empty");

accessor.writeValue("true");
assert.strictEqual(true, accessor.view.checked);

accessor.writeValue("blah");
assert.strictEqual(false, accessor.view.checked, "default to false on parse errors");
assert.throws(() => accessor.writeValue("blah"));
});

it("converts strings to dates", () => {
const now = new Date();
const accessor = new TestDateValueAccessor();

accessor.writeValue(null);
assert.equal(formatDate(now), formatDate(accessor.view.date), "default to now on empty");
assert.equal(null, accessor.view.date, "default to null on empty");

accessor.writeValue("2010-03-17");
assert.equal(formatDate(new Date(2010, 2, 17)), formatDate(accessor.view.date));

accessor.writeValue("a fortnight ago");
assert.equal(formatDate(now), formatDate(accessor.view.date), "default to now on parse error");
});

it("converts strings to int selection", () => {
const accessor = new TestSelectedIndexValueAccessor();

accessor.writeValue(null);
accessor.ngAfterViewInit();
assert.strictEqual(0, accessor.view.selectedIndex, "default to 0 on empty");
assert.strictEqual(null, accessor.view.selectedIndex, "default to null on empty");

accessor.writeValue("3");
accessor.ngAfterViewInit();
assert.strictEqual(3, accessor.view.selectedIndex);

accessor.writeValue("blah");
accessor.ngAfterViewInit();
assert.strictEqual(0, accessor.view.selectedIndex, "default to 0 on parse errors");
assert.notEqual(accessor.view.selectedIndex, accessor.view.selectedIndex,
"default to NaN on parse errors");
});

it("converts strings to times", () => {
const now = new Date();
const accessor = new TestTimeValueAccessor();

accessor.writeValue(null);
assert.equal(formatTime(now), formatTime(accessor.view.time), "default to now on empty");

accessor.writeValue("2010/03/17 12:54");
assert.equal(formatTime(new Date(2010, 2, 17, 12, 54)), formatTime(accessor.view.time));

accessor.writeValue("three hours from now");
assert.equal(formatTime(now), formatTime(accessor.view.time), "default to now on parse error");
assert.throws(() => accessor.writeValue(null));
assert.throws(() => accessor.writeValue("2010/03/17 12:54"));
assert.throws(() => accessor.writeValue("three hours from now"));
});

it("converts values to text", () => {
const now = new Date();
const accessor = new TestTextValueAccessor();

accessor.writeValue(null);
assert.equal("", accessor.view.text);
assert.equal(null, accessor.view.text, "defaults to null on empty");

accessor.writeValue("blah");
assert.equal("blah", accessor.view.text);
Expand Down
3 changes: 2 additions & 1 deletion tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"appium-android": "tns build android && npm run run-appium-android",
"run-appium-android": "nativescript-dev-appium android",
"appium-ios-simulator": "tns build ios && nativescript-dev-appium ios-simulator",
"tslint": "tslint --project tsconfig.json --config tslint.json"
"tslint": "tslint --project tsconfig.json --config tslint.json",
"appium": "nativescript-dev-appium"
}
}
1 change: 0 additions & 1 deletion tests/references.d.ts

This file was deleted.