Expose new public API in light of recent spec changes #36
Conversation
domenic
force-pushed
the
update-everything
branch
from
fc5ed98 to
5fe80e2
Compare
domenic
force-pushed
the
update-everything
branch
from
5fe80e2 to
762c154
Compare
| throw new TypeError("Failed to construct 'URL': 1 argument required, but only 0 present."); | ||
| } | ||
| module.exports.createURLConstructor = function () { | ||
| // TODO |
There was a problem hiding this comment.
We should probably put a util method for this into webidl2js.
There was a problem hiding this comment.
Hmm actually I think this is legacy; createURLConstructor is no longer necessary since we just export the URL constructor directly.
There was a problem hiding this comment.
We want to be sure someone can't overwrite objects for all contexts though - although that's a more general jsdom problem (which we might have the opportunity to solve with webidl2js while we're at it).
|
Haven't looked at the actual spec stuff yet, but are you sure about the state dict to string conversion? Imo mistyping the string is much easier than using auto-complete from the state dict - we could even put the strings in the state dict to keep the easier debugging. |
| "request": "^2.55.0", | ||
| "recast": "~0.10.29" | ||
| "webidl2js": "jsdom/webidl2js" |
There was a problem hiding this comment.
Gotta remember to switch this when I release 3.0
My main concern is that the public API should be using the strings, since that is idiomatic JS for enumerations. (We could validate them there if that'd be preferred.) Internally I just used strings everywhere since then we don't have a bunch of annoying mapping code; IMO the payoff of such mapping code is not worth the cost, both in authoring and in maintenance. But if you'd really prefer it for IDE reasons or similar than I am OK with that. |
domenic
force-pushed
the
update-everything
branch
from
762c154 to
28557b2
Compare
|
OK, this is ready for review, and I've validated it suffices for jsdom. |
| } | ||
|
|
||
| static domainToASCII(domain) { | ||
| try { |
There was a problem hiding this comment.
Looks like parseHost can't throw anymore so I don't think we need this anymore, right?
There was a problem hiding this comment.
Oooh, it does "return failure" instead, I see. Eek.
|
Okay, through with this one at least. |
domenic
force-pushed
the
update-everything
branch
from
28557b2 to
4e2f5a5
Compare
|
Pushed revised version with feedback incorporated plus an additional commit for recent URL spec changes. Tests fail until tr46 is fixed and WPT is merged. |
The web platform tests have been updated to a nicer format, so we adapt. Also includes a script used to update the web platform tests.
This will cause the tests to fail until web-platform-tests/wpt#2500 is merged.
domenic
force-pushed
the
update-everything
branch
from
4e2f5a5 to
d1dc306
Compare
This exposes the URL constructor, which is now re-written to use webidl2js and be based on the latest spec. It also exposes a bunch of internal methods I needed to make jsdom work.
Not quite ready to merge, but ready to review I think. Blocked on jsdom/webidl-conversions#3 and jsdom/webidl2js#10. Also, a bunch of tests fail, but I am pretty sure they are all due to web platform tests being outdated versus the spec.
Separate jsdom PR coming up that uses these now-exposed hooks.