Skip to content
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

More friendly messages for non-string #1

Merged
merged 4 commits into from
Mar 30, 2018

Conversation

Hoishin
Copy link
Contributor

@Hoishin Hoishin commented Feb 14, 2018

(Originally opened at sindresorhus/parse-json#16)

When I passed undefined, the error was

TypeError: Cannot read property 'length' of undefined

For NaN, Infinity, new Set() etc,

TypeError: txt.slice is not a function

The cause was assuming the passed value is a string, so I added type check. Throws message with the type and the value converted to string if the value is not string.

@Hoishin
Copy link
Contributor Author

Hoishin commented Feb 19, 2018

After some discussion on sindresorhus/parse-json#16, I updated error handling.

  • If txt is an empty array [], it special-cases and says Cannot parse an empty array, since String([]) is an empty string and would be confusing.
  • Other non-strings are converted to string with String().
  • Throws TypeError

Also added a bunch of test codes to increase coverage to 100%.

@Hoishin
Copy link
Contributor Author

Hoishin commented Mar 30, 2018

sorry to ping @zkat , I'd appriciate if you could review this PR :)

Copy link
Owner

@zkat zkat left a comment

Choose a reason for hiding this comment

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

seems reasonable enough

@zkat zkat merged commit a476d42 into zkat:latest Mar 30, 2018
@zkat
Copy link
Owner

zkat commented Mar 30, 2018

Merged and released as v1.0.2. Thanks for the contrib!

@Hoishin Hoishin deleted the fix-anbiguous-error branch March 30, 2018 18:12
iarna added a commit to npm/npm that referenced this pull request Apr 9, 2018
More friendly error messages.

PR-URL: zkat/json-parse-better-errors#1
Credit: @Hoishin
iarna added a commit to npm/npm that referenced this pull request Apr 10, 2018
More friendly error messages.

PR-URL: zkat/json-parse-better-errors#1
Credit: @Hoishin
iarna added a commit to npm/npm that referenced this pull request Apr 10, 2018
More friendly error messages.

PR-URL: zkat/json-parse-better-errors#1
Credit: @Hoishin
iarna added a commit to npm/npm that referenced this pull request Apr 11, 2018
More friendly error messages.

PR-URL: zkat/json-parse-better-errors#1
Credit: @Hoishin
iarna added a commit to npm/npm that referenced this pull request Apr 11, 2018
More friendly error messages.

PR-URL: zkat/json-parse-better-errors#1
Credit: @Hoishin
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.

2 participants