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

fix: support for Infinity && NaN #26

Conversation

richard-livingston
Copy link
Contributor

Why?

  • If the JSON5 file contains Infinity or NaN they will be converted to null by JSON5.stringify, making the required JSON5 file unusable.

How?

  • Use util.inspect instead of JSON5.stringify, preserving types not allowed in native JSON such as Infinity and NaN.

Why?
 - If the JSON5 file contains Infinity or NaN they will be converted to null by JSON5.stringify, making the required JSON5 file unusable.

How?
 - Use util.inspect instead of JSON5.stringify, preserving types not allowed in native JSON such as Infinity and NaN.
@jsf-clabot
Copy link

jsf-clabot commented Jul 11, 2017

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Need tests

 - Output is formatted with spaces instead of tabs, without new lines, and with single quotes.
@codecov
Copy link

codecov bot commented Jul 11, 2017

Codecov Report

Merging #26 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #26   +/-   ##
=======================================
  Coverage   83.33%   83.33%           
=======================================
  Files           2        2           
  Lines           6        6           
=======================================
  Hits            5        5           
  Misses          1        1
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebb370d...850bce2. Read the comment docs.

@richard-livingston
Copy link
Contributor Author

Hi @evilebottnawi I've added some tests

@alexander-akait
Copy link
Member

@richard-livingston seems cla buggy, just close and reopen PR

@richard-livingston
Copy link
Contributor Author

@michael-ciniawsky You marked this pull request as a feature, but I'm not sure it is, this is functionality already supported by JSON5 but not by this loader. This is the core functionality.

@richard-livingston
Copy link
Contributor Author

@evilebottnawi ahh, I've been trying to figure that cla out for the last 30 mins. You're very fast :)

@michael-ciniawsky michael-ciniawsky changed the title feat: support Infinity & NaN fix: support Infinity & NaN Jul 11, 2017
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

Richard Livingston seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

😛 You need to sync the email used by Git with Github or vice versa && fix the commit author

@richard-livingston
Copy link
Contributor Author

Thanks Michael, will do as soon as I can

@richard-livingston
Copy link
Contributor Author

Hi @michael-ciniawsky It seems to have fixed itself now:

All committers have signed the CLA.

@michael-ciniawsky michael-ciniawsky merged commit 5a8ca43 into webpack-contrib:master Jul 12, 2017
@michael-ciniawsky michael-ciniawsky changed the title fix: support Infinity & NaN fix: support for Infinity && NaN Jul 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants