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

Expose initial JSON.parse as %JSONParse% #1012

Closed
annevk opened this issue Sep 28, 2017 · 7 comments
Closed

Expose initial JSON.parse as %JSONParse% #1012

annevk opened this issue Sep 28, 2017 · 7 comments

Comments

@annevk
Copy link
Member

annevk commented Sep 28, 2017

That way Fetch and XMLHttpRequest can use it more directly (and other web platform entry points that need JSON parsing). Alternative is an abstract operation, which might be a little clearer usage wise.

Any preference?

@annevk
Copy link
Member Author

annevk commented Sep 28, 2017

I missed that %JSON% already exists. Could I write something like

Return ? Invoke(%JSON%, "parse", « jsonSource »).

then?

@michaelficarra
Copy link
Member

I'd prefer we just name the intrinsic so you can call it directly.

@ljharb
Copy link
Member

ljharb commented Sep 28, 2017

With the Invoke, you'd be calling whatever I happened to install on the JSON object under "parse"; it definitely would need a direct name.

@annevk
Copy link
Member Author

annevk commented Sep 28, 2017

Thanks, I'll work on a PR. How would I end up using it in prose afterwards? Just

Return ? %JSONParse%( jsonSource )

or something more involved?

@ljharb
Copy link
Member

ljharb commented Sep 28, 2017

That seems right to me!

@domenic
Copy link
Member

domenic commented Sep 28, 2017

Return ? %JSONParse%( jsonSource )

No, that's not correct. That's only correct if it's an abstract operation ("JSONParse", no %s), which indeed is neater.

If we expose it as a named intrinsic, we'd need

Return ? Call(%JSONParse%, undefined, « jsonSource »).

I think an abstract operation would be nicer if we can get away with it.

If we do that we can also make it encompass only steps 2-6 of the JSON.parse function.

bterlson pushed a commit that referenced this issue Sep 28, 2017
* Add %JSONParse% for Fetch et al

Fixes #1012.

* add <dfn>
@annevk
Copy link
Member Author

annevk commented Sep 28, 2017

Okay, I think we can just add an abstraction in Infra to make the difficulty go away for everyone else in the web platform. (And maybe also require UTF-8 there at the same time when the input is bytes.)

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

No branches or pull requests

4 participants