-
Notifications
You must be signed in to change notification settings - Fork 139
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
Define URL's toJSON() #229
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2483,6 +2483,8 @@ interface URL { | |
attribute USVString search; | ||
[SameObject] readonly attribute URLSearchParams searchParams; | ||
attribute USVString hash; | ||
|
||
USVString toJSON(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If “URL” interface will be implemented on PHP, should this method be implemented by the name of “jsonSerialize()” instead of “toJSON()”? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're porting the JavaScript URL class to PHP, you can do whatever you like, as there's no spec for that. It's up to you what you think is most intuitive for your developers (e.g. probably PHP has different naming conventions than JavaScript in many areas, not just JSON serialization). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get it. Thank you for your advice. |
||
}; | ||
</pre> | ||
|
||
|
@@ -2579,8 +2581,9 @@ url.pathname // "/%F0%9F%8F%B3%EF%B8%8F%E2%80%8D%F0%9F%8C%88"</pre> | |
|
||
<h3 id=urlutils-members>{{URL}} members</h3> | ||
|
||
<p>The <dfn attribute for=URL><code>href</code></dfn> attribute's getter must return the | ||
<a lt='URL serializer'>serialization</a> of <a>context object</a>'s <a for=URL>url</a>. | ||
<p>The <dfn attribute for=URL><code>href</code></dfn> attribute's getter and the | ||
<dfn method for=URL>toJSON()</dfn> method, when invoked, must return the | ||
<a lt="URL serializer">serialization</a> of <a>context object</a>'s <a for=URL>url</a>. | ||
|
||
<p>The <code><a attribute for=URL>href</a></code> attribute's setter must run these steps: | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though I'm not a fan of the complicated serializer syntax, I do like the conciseness of just writing
serializer;
which is more parallel to the existing
iterable<USVString, USVString>;
forURLSearchParams
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be, but this is the way IDL is going for
toJSON()
. I checked with @bzbarsky and @tobie.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the question one of having
serializer;
basically be a thing that produces a then-custom-defined toJSON, kinda likestringifier;
?The problem is that unlike
stringifier;
, where the return value is clear, the return value of toJSON could be all sorts of things. We could defineserializer;
to induce the equivalent ofany toJSON();
but that's more annoying to work with than being able to usefully declare your return type...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I haven't thought of that. How about
serializer<USVString>;
? It also has the advantage of being more language-independent, while there are precedents for<>
meaning "produces" withiterable
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? We should be moving away from this baroque, hard-to-understand, "language independent" syntax in favor of simple, do-what-it-says JavaScript-derived syntax.
The only reason iterable is special is because it's a macro that expands to ~4 JS methods. For a simple stand-in for a JS method, we should not create an extra layer of abstraction that requires looking up the Web IDL spec to understand what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to assume @TimothyGu is okay with the PR as-is given five days have passed. Hope that's okay. (Still I'll wait a little longer with landing since the test required a further change.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annevk, sorry about forgetting to reply. I'm okay with the PR in general. The bit about IDL notation is minor and does not affect the semantics anyway.