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

help wanted: Web APIs' implementation #1047

Open
kyranet opened this Issue Oct 21, 2018 · 40 comments

Comments

@kyranet
Copy link
Contributor

kyranet commented Oct 21, 2018

Previously discussed in #82, this is a meta issue to help contributors (and future ones) track some of the Web APIs in deno.

Note: The HTTP web server first will be done before WebSocket's initial development. However, WebSocket extends EventTarget which inherently requires Event. Worker also extends EventTarget.

Additionally, we could later implement the Web Audio API for audio processing, and the HTML Canvas 2D Context API for image processing/generation, too.

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 21, 2018

I'd like to help! Are there other examples of specs that have been implemented so far to use as a reference point for these? I'd be interested to tackle the event or URL ones.

@kyranet

This comment has been minimized.

Copy link
Contributor Author

kyranet commented Oct 21, 2018

I believe that the MDN docs and the WHATWG ones should suffice, they're also available in any modern browser so we can play around with those APIs.

There was a version of URL in the golang version but it was removed in the rust rewrite. I'm currently working on URLSearchParams and later URL, but we can try to cooperate if you want 👍

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 21, 2018

Perfect! I can take a look at Events then, although I will say this is pretty intimidating, that Golang spec is almost 700 lines

@ztplz

This comment has been minimized.

Copy link
Contributor

ztplz commented Oct 21, 2018

I am working on EventTarget.

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Oct 21, 2018

A good example of Web APIs implemented in plain JavaScript is jsdom.

Most of the APIs we are talking about have interfaces in TypeScript which we should follow. For example EventTarget.

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 21, 2018

@kyranet thanks for putting out #1049 that actually gives me a good guideline for following along for Event. I'll give that one a shot.

@kyranet

This comment has been minimized.

Copy link
Contributor Author

kyranet commented Oct 21, 2018

I'm glad to know it helped you! I'll possibly leave URL for somebody else, it's far more complex than URLSearchParams and my free time is currently lacking. Feel free to pick it up after Event 👍

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

@kyranet I couldn't comment on the merged PR but shouldn't the constructor for the URL search params be init?: since the spec mentions it as optional?

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

I've implemented a rough draft of Events in #1059 but I think it's blocked by @ztplz 's EventTarget implementation because a lot of the properties rely on an EventTarget object. It would be great to get some help on that since I'm a bit confused by a few things.

@acconrad acconrad referenced this issue Oct 22, 2018

Merged

Web API: Event #1059

5 of 5 tasks complete
@vladfrangu

This comment has been minimized.

Copy link

vladfrangu commented Oct 22, 2018

I'm willing to give a shot at implementing the URL spec, if that's ok with everyone. I'll probably start by inspiring and / or porting the golang implementation.

@kyranet

This comment has been minimized.

Copy link
Contributor Author

kyranet commented Oct 22, 2018

@acconrad

@kyranet I couldn't comment on the merged PR but shouldn't the constructor for the URL search params be init?: since the spec mentions it as optional?

It's actually optional, = "" implies it, tslint complained when I used both init?: and = "", suggesting to use init: instead:

constructor(init: string | string[][] | Record<string, string> = "") {

From WHATWG: https://url.spec.whatwg.org/#interface-urlsearchparams

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Oct 22, 2018

Use node poly fill modules where possible - no need to reinvent the wheel. With URL there is already a node module.

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

For everyone here if you're confused by the interface definitions within the WhatWG, check out this doc on the official Interface Document Language :)

@ztplz

This comment has been minimized.

Copy link
Contributor

ztplz commented Oct 22, 2018

@acconrad Sorry for my delay PR. Github still is down, I will do it next day.

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

@ztplz don't worry, I'm not in a rush, I still need to do a final draft of the Event API since I'm still learning how the spec operates myself :)

We will need to coordinate implementations though because I have flags within Event that you'll need to make this all work

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

I'll grab Worker next after Event/EventInit btw - @kyranet can you update the issue description to show who has claimed what?

@kyranet

This comment has been minimized.

Copy link
Contributor Author

kyranet commented Oct 22, 2018

Just to clarify, add EventInit and assign both Event and EventInit to @acconrad, then assign EventTarget to @ztplz?

Worker is going to be pretty interesting to implement, I'm looking forward to seeing how it will be implemented in deno 😄

@mreinstein

This comment has been minimized.

Copy link
Contributor

mreinstein commented Oct 22, 2018

@ry deno's philosophy of following browser APIs has a lot going for it, and I've been keen to see this play out in the development. One question I have is how rigidly are we following this model? a few points:

  • having near-perfect browser API compatibility would be awesome, avoiding node-isms are one of the big draws of this project (commonjs, etc)
  • a handful of web APIs are an unmitigated disaster (e.g., the audio APIs are just a complete mess.)
  • I weep to think of our team spending time perfectly mimicking truly awful APIs. It seems like such a wasted opportunity to get something like audio right.

So I guess my question is does it make sense to replicate APIs that are really suffering from poor design? I understand the tradeoffs, I'd like to understand this project's philosophies.

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Oct 22, 2018

@mreinstein I agree that not all web APIs are created equal. For example, I do not consider the stream APIs sufficient for general purpose high concurrency servers - thus we are providing the lower-level deno.Reader/deno.Writer interfaces. Another example, the web "File" API does not sufficiently cover the myriad of posix File I/O - we provide a lower-level version of this. In both cases, I think Deno will provide the web APIs but implement them in terms of the lower-level, more generalized APIs.

I don't know anything about the Audio API - it seems that's something very far off for us to consider at the moment...

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

@mreinstein @ry would be good to know what level of compliance you are looking for as it relates to the Web IDL standard I referenced. For example, readonly in TypeScript does not mean the same thing as readonly in the Web IDL, which really means:

This prototype property is { get: propGetter, configurable: true, enumerable: true }

And to be truly compliant, you have to use WeakMaps for private state. Again, that's just one example but it's useful to know for both security reasons and quality standards.

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

@kyranet :

Just to clarify, add EventInit and assign both Event and EventInit to @acconrad, then assign EventTarget to @ztplz?

Worker is going to be pretty interesting to implement, I'm looking forward to seeing how it will be implemented in deno 😄

Yeah that's just one example, I know someone else claimed URL too so it would just be good to see what is claimed and who claimed it so we can figure out what is left to tackle

@ztplz

This comment has been minimized.

Copy link
Contributor

ztplz commented Oct 22, 2018

@ry Do we need to add more type declarations for web APIs? Element is not defined yet in dom_type.ts.

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Oct 22, 2018

@acconrad Regarding level of compliance - I don't have any particular opinions on this. Web scripts should run on Deno if they are using APIs we support - with little or no modification - that's all I care about. If you feel strongly about how this should be done, perhaps add some text to the contributing guide?

@ztplz Let's add them as necessary to support specific functionality that you need. I don't want to add a bunch of interfaces for APIs that are not usable. That goes for these above APIs here - I'm fine adding EventTarget - if it's needed for some specific functionality - but let's not add abstract utilities that are not used anywhere. There are many many web APIs - we are not trying to reimplement a complete headless chrome - so please add interfaces lazily.

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Oct 22, 2018

@kyranet For URL, I used https://github.com/denoland/deno/blob/golang/url.js in the prototype.
If you can use it without forking and just putting it into package.json, that would be great. We should reduce the amount of code we have to maintain as much as possible. I forget why I forked it originally.

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Oct 22, 2018

That URL implementation is clean and modern JavaScript. My feeling is something like that could be included cleanly in Deno. Some of the other polyfills aren't written as cleanly or have a lot of code to support older browsers, or rely on Node.js APIs, etc.

It is a fine balance between less maintaining of code and not dragging in a whole load of stuff and keeping type safety.

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Oct 22, 2018

I should also point out that there is already a free test suite out there for reaching Web API compliance:

https://github.com/web-platform-tests/wpt

We should try to leverage these with each of the interfaces we are implementing

@mrkurt

This comment has been minimized.

Copy link

mrkurt commented Nov 2, 2018

We have a pretty clean URL implementation we can do a PR for if you all want. The URL polyfills have been a headache for us over the last 8 mo:

https://github.com/superfly/fly.rs/blob/master/v8env/src/url.ts

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Nov 2, 2018

@mrkurt that would be appreciated - code looks good

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Nov 5, 2018

@ztplz any updates on EventTarget? do you have something or have you not started yet? i've realized that Event and EventTarget are very closely linked so it may make sense to do both at once (which then makes me think CustomEvent would be next after that). i was going to grab Worker next, maybe you could take that instead?

@watilde

This comment has been minimized.

Copy link
Contributor

watilde commented Nov 17, 2018

To follow URL spec in WHATWG, this one would be one of the ideal packages to put :)
https://github.com/jsdom/whatwg-url/

@ruphin

This comment has been minimized.

Copy link
Contributor

ruphin commented Dec 3, 2018

Is the Web Crypto API on the list of Web APIs we want to implement? There are excellent user space JS alternatives for most of endpoints in this API, but we need some cryptographically secure random source which cannot be done in user space.

@kyranet

This comment has been minimized.

Copy link
Contributor Author

kyranet commented Dec 3, 2018

Most likely, yes @ruphin, cryptography is pretty important and I think we should support it.

@ruphin

This comment has been minimized.

Copy link
Contributor

ruphin commented Dec 3, 2018

I need a secure random source for something I'm working on. I can take a stab at implementing Crypto.getRandomValues https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues

@hayd

This comment has been minimized.

Copy link
Contributor

hayd commented Dec 3, 2018

Ideally webcrypto can be added to the build, rather than adding functionality ad hoc
https://www.chromium.org/blink/webcrypto

@mrkurt

This comment has been minimized.

Copy link

mrkurt commented Dec 3, 2018

@watilde the jsdom whatwg-url package leaks memory like a fiend (to the point where we were blowing up isolates with 256MB of RAM allocated trying to use it).

@kitsonk kitsonk referenced this issue Dec 17, 2018

Merged

Add URL #1359

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Jan 6, 2019

@kyranet you can update this. @kitsonk added URL and I added in Event and EventTarget. I'm going to next work on improving the EventTarget implementation and then move onto CustomEvent after that.

After that I think only WebWorker and WebSocket are left - what's the plan after these are all implemented?

@kevinkassimo

This comment has been minimized.

Copy link
Contributor

kevinkassimo commented Jan 6, 2019

@acconrad Ryan is working on workers alongside with native ES module (since the TS compiler would be soon moved to a worker).

@hayd

This comment has been minimized.

Copy link
Contributor

hayd commented Jan 6, 2019

@acconrad and websockets are in deno_std now denoland/deno_std#84

@kevinkassimo

This comment has been minimized.

Copy link
Contributor

kevinkassimo commented Jan 6, 2019

@hayd I think we could possibly move client side WebSocket APIs from deno_std to the main repo as a global after a more careful review

@acconrad

This comment has been minimized.

Copy link
Contributor

acconrad commented Jan 6, 2019

Nice! So really events are the last of it this sounds like! I'll be sure to follow-up with EventTarget (finalized) and CustomEvent asap!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment