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

Improvements covering a few ongoing issues #89

Merged
merged 8 commits into from Jan 6, 2020
Merged

Conversation

rricard
Copy link
Member

@rricard rricard commented Dec 24, 2019

This change fixes #40, fixes #74, fixes #88.

If anything is too controversial, I can isolate a commit and discuss it further in an another PR.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Robin Ricard and others added 3 commits December 28, 2019 11:00
Adds # on each parsing/runtime error example

Co-Authored-By: Jordan Harband <ljharb@gmail.com>
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@rickbutton rickbutton merged commit e70e38f into master Jan 6, 2020
const r2 = #{ obj: true ? {} : #{} }; // will fail at runtime as it's not obvious from the parser's perspective that we should fail
const r3 = #{ method() {} }; // fails at parse time
```
If you try to use a method definitions as part of a Record, the same behavior is expected, it should fail at runtime (even if we could fail at compile time).
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why you removed the examples

README.md Outdated
@@ -344,10 +338,6 @@ We propose to add `JSON.parseImmutable` so we can extract a Record/Tuple type ou

The signature of `JSON.parseImmutable` is identical to `JSON.parse` with the only change being in the return type that is now a Record or a Tuple.

## JSON.stringify
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why this was removed either; it's good to know, even if there's zero spec text corresponding to it.

Copy link
Member

Choose a reason for hiding this comment

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

It was already specified somewhere else in the file, so this was a duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants