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

Show the line where JSON.parse throw an error #4314

Closed
wants to merge 1 commit into from
Closed

Show the line where JSON.parse throw an error #4314

wants to merge 1 commit into from

Conversation

rianby64
Copy link
Contributor

@rianby64 rianby64 commented Dec 12, 2016

Currently in Chrome and Opera it's not working as expected.
I want to see the line that fired the following error:

Uncaught (in promise) SyntaxError: Unexpected end of JSON input

Firefox shows correctly the line where this error was originated.

Currently in Chrome and Opera it's not working as expected.
I want to see the line that fired the following error:

  Uncaught (in promise) SyntaxError: Unexpected end of JSON input

Firefox shows correctly the line where this error was originated.
@rianby64
Copy link
Contributor Author

I've some questions about this test

  1. Where's a test for JSON.parse?
  2. Did I place the test in the correct place?
  3. Is it useful?

@zcorpan
Copy link
Member

zcorpan commented Dec 12, 2016

  1. https://github.com/tc39/test262/search?utf8=✓&q=json.parse

For (2) and (3), while having line numbers certainly seems like a useful thing, the spec for this feature currently has issues. There are also a bazillion things that can cause an error, and it doesn't scale well to copy-paste this test for each thing. The existing tests are mostly trying to cover each aspect of the HTML spec's requirements, but not testing each possible scenario for each requirement.

That said, testing multiple scenarios and in particular cases people run into that are a problem for them is great to have tests for. So I think:

  • The existing test(s) should be modified to also test exact line (and column?), but those should probably be new separate test()s.
  • We should figure out a way to test several scenarios for each test in a DRY way, so it's reasonably easy to maintain the tests. Maybe a build script could work? (Maybe this could be a follow-up though; if so, please file a new issue.)

@foolip
Copy link
Member

foolip commented Oct 28, 2019

Closing due to no submitter response.

@foolip foolip closed this Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants