Skip to content

Conversation

@rayners
Copy link

@rayners rayners commented Oct 9, 2019

I noticed that the default and defaultValue properties are applied from functions in the case of null or undefined (see https://github.com/zemirco/json2csv/blob/v4/lib/JSON2CSVBase.js#L81), but only undefined for simple properties (https://github.com/zemirco/json2csv/blob/v4/lib/utils.js#L4). I also noticed that the tests for defaultValue were accidentally relying on the null -> "" transformation, so I updated them to exercise the behavior better.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 2b94f28 on rayners:ensure-null-gets-defaultValue into d891c64 on zemirco:v4.

@knownasilya
Copy link
Collaborator

Awesome, love when tests are submitted with a fix!

@knownasilya knownasilya merged commit 0a01470 into zemirco:v4 Oct 9, 2019
@knownasilya
Copy link
Collaborator

Released as v4 patch

@juanjoDiaz
Copy link
Collaborator

A bit late to the party but, I have a couple comments/questions:

null is not the same as undefined so I'm not 100% sure if we should consider it as undefined. But that's ok.

More importantly, what is the reason to change the tests to use a default value of '-' instead of ''?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants