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

[BUG] Transform `{NaN,Infitity,-Infinity}` to null #2513

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@seanpdoyle
Contributor

seanpdoyle commented Nov 25, 2014

#2509 (comment)

Serializing and Deserializing invalid numeric JSON (NaN,
Infinity, -Infinity) should result in null.

fivetanley and others added some commits Nov 25, 2014

Merge pull request #2511 from emberjs/more-ie8-oo-create-fixes
make the Object.create assertion more specific
Merge pull request #2509 from seanpdoyle/add-tests-number-transform
Add Test Coverage for `DS.NumberTransform`
@@ -2,6 +2,10 @@ import Transform from "ember-data/transforms/base";
var empty = Ember.isEmpty;
function isNumber(value) {
return !isNaN(value) && value !== Infinity && value !== -Infinity;

This comment has been minimized.

@stefanpenner

stefanpenner Nov 25, 2014

Member

for some reason my brain is telling me isNaN should be avoided, instead the test should be value !== value. That maybe just be a perf concern.

[BUG] Transform `{NaN,Infitity,-Infinity}` to null
#2509 (comment)

Serializing and Deserializing invalid numeric `JSON` (`NaN`,
`Infinity`, `-Infinity`) should result in `null`.

@seanpdoyle seanpdoyle force-pushed the seanpdoyle:number-transform-nan-and-infinities branch from 1dbe546 to 7bfdb1b Nov 25, 2014

@seanpdoyle

This comment has been minimized.

Contributor

seanpdoyle commented Nov 25, 2014

@stefanpenner swapped !isNaN with === 👍

@@ -2,6 +2,10 @@ import Transform from "ember-data/transforms/base";
var empty = Ember.isEmpty;
function isNumber(value) {
return value === value && value !== Infinity && value !== -Infinity;

This comment has been minimized.

@stefanpenner

stefanpenner Nov 25, 2014

Member

value !== value

This comment has been minimized.

@seanpdoyle

seanpdoyle Nov 25, 2014

Contributor

We're checking for the value's validity.

NaN === NaN // => false
1 === 1     // => true

@igorT igorT force-pushed the emberjs:master branch from b38d12f to 894457a Nov 26, 2014

@fivetanley

This comment has been minimized.

Member

fivetanley commented Nov 27, 2014

Merged via cherry-pick: 6c3d23d

Thanks!

@fivetanley fivetanley closed this Nov 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment