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

Publish the web-sys crate #613

Closed
17 of 19 tasks
fitzgen opened this issue Aug 2, 2018 · 34 comments
Closed
17 of 19 tasks

Publish the web-sys crate #613

fitzgen opened this issue Aug 2, 2018 · 34 comments
Labels
frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen web-sys Issues related to the `web-sys` crate

Comments

@fitzgen
Copy link
Member

fitzgen commented Aug 2, 2018

As discussed in today's WG meeting, we are aiming for publishing an initial release of the web-sys crate for the Rust 2018 Release Candidate milestone. That's 6 weeks from now: 2018-09-13.

What needs to be done before we ship an initial release of web-sys?

Goals

Stretch


What else should we make sure is done for an initial release? For reference, here are all the "frontend:webidl" issues and here are all the "web-sys" issues

+cc @ohanar @alexcrichton @twilco @Dodj @jonathanKingston

@fitzgen fitzgen added frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen web-sys Issues related to the `web-sys` crate Blocking Rust 2018 labels Aug 2, 2018
@ohanar
Copy link
Member

ohanar commented Aug 2, 2018

Namespaces and inheritance have been on the top of my todo list when I get a moment. Inheritance is needed for many useful bindings (my plan for the moment was to basically treat them like mixins until we decide on how we want to expose the hierarchy).

@fitzgen
Copy link
Member Author

fitzgen commented Aug 2, 2018

@ohanar good call! Added an item about implementing inheritance from the RFC.

I think we should hold off on mixins or traits for base methods until we have another accepted RFC, since we definitely don't want to ship something as big as that until we are fairly sure it will be stable and not undergo a bunch of churn. That means that if we want to include something like that for the initial web-sys release, we should be starting design work / writing a draft of that RFC now.

@alexcrichton
Copy link
Contributor

Another checklist item (which I don't think we have a tracking issue for yet) is a lack of constructors, for example HtmlDivElement doesn't have any constructor methods generated, but the WebIDL lists the HTMLConstructor custom attribute, which I believe we're not currently processing.

@alexcrichton
Copy link
Contributor

One last thing I think we'll want to do is to set up headless testing for Chrome as well on CI to ensure that we don't get things that are too Firefox-specific

@ohanar
Copy link
Member

ohanar commented Aug 3, 2018

@fitzgen I would definitely say the inheritance RFC needs to happen then. The most popular proposal of using traits (like stdweb), doesn't match the current API, since all methods would be on traits rather than directly on the types, so implementing such a proposal would require comparable churn.

@ohanar
Copy link
Member

ohanar commented Aug 3, 2018

One last thing that I just thought of, is that typedefs should be removed. The spec makes it clear that these should not be exposed in language bindings. With the first pass in the webidl frontend, it should be quite straightforward to process them there.

@twilco
Copy link
Contributor

twilco commented Aug 3, 2018

Do we want to specifically call out #248 as a checklist item? It is blocking generation of methods for certain structs - e.g., the Storage struct, which has a getter and setter defined in the WebIDL.

@alexcrichton
Copy link
Contributor

I've updated the OP and filed dedicated issues for the items mentioned here!

@fitzgen
Copy link
Member Author

fitzgen commented Aug 3, 2018

@ohanar

The most popular proposal of using traits (like stdweb), doesn't match the current API, since all methods would be on traits rather than directly on the types, so implementing such a proposal would require comparable churn.

Couple things:

  1. We definitely need a more fleshed out design than "using traits like stdweb" ;) If you want to start a draft/WIP RFC that we can collaborate on and discuss, that would be super helpful in pushing this forward!

  2. I could see a potential universe where we shipped without any base-traits inheritance method stuff, only shipped with normal methods, and then as a non-breaking bump added the traits as additional things. That said, I would still prefer to ship with the nicer traits-based thing.

@ohanar
Copy link
Member

ohanar commented Aug 5, 2018

@fitzgen

I would be happy to write up a draft RFC, although I'm super swamped at the moment and likely won't have much time to do anything real for at least a week.

The main complaint that I have with a non-breaking traits based thing is that it feels super weird to me for instance for a MouseEvent to have have a related_target property (in the form of a getter), but not have a target property unless a trait is imported. I worry that the asymmetry would be confusing to newcomers. (I also worry that a whole trait-based approach would be unwelcoming for people coming from the javascript world where nothing extra has to be done to access base class methods/properties.)

@Pauan
Copy link
Contributor

Pauan commented Aug 5, 2018

I would be happy to write up a draft RFC, although I'm super swamped at the moment and likely won't have much time to do anything real for at least a week.

If nobody else wants to, I can write up a trait-inheritance RFC.

(I also worry that a whole trait-based approach would be unwelcoming for people coming from the javascript world where nothing extra has to be done to access base class methods/properties.)

I can sympathize with that, but I think that will be the least of concerns for a JavaScript user trying to learn Rust (they will struggle much more with other stuff).

Especially since it's as easy as just slapping use web_sys::traits::* at the top of your file and you're good to go.

@jonathanKingston
Copy link
Contributor

jonathanKingston commented Aug 5, 2018

is a lack of constructors, for example HtmlDivElement doesn't have any constructor methods generated
The only way that I know of to create these are through document.createElement

Currently the crate doesn't support the or webidl keyword in argument position:

  Element createElement(DOMString localName, optional (ElementCreationOptions or DOMString) options);

So my priorities would be:

  • Supporting document.create_element
  • Exposing document: Document or a simple way to create without using #[wasm_bindgen]
  • Allowing upcasting create_element() result to HtmlDivElement
  • Implement iterator support for things like NodeList.
  • Provide a simple example of web_sys building up a form or table and inserting it into body().

@alexcrichton
Copy link
Contributor

@jonathanKingston ah excellent points! The or keyword is the "union" type I believe which is currently unsupported. We don't really have a way to express union types in a great way (that works well with wasm-bindgen), so what I think we should do is have a pass over the WebIDL which basically "expands" union types. For example the method:

  Element createElement(DOMString localName, optional (ElementCreationOptions or DOMString) options);

would become

  Element createElement(DOMString localName, optional DOMString options);
  Element createElement(DOMString localName, optional ElementCreationOptions options);

we already have code to handle overloads and such, so that way we wouldn't have to deal with unions and we'd just export a function-per-element of the union. AFAIK unions can't be nested as well, so this should be a semi-local pass.

@afdw
Copy link
Contributor

afdw commented Aug 5, 2018

Yes, but what to do with unions in other positions (for example, return types)? Looks like supporting it would require expressing them as Rust enums.

@alexcrichton
Copy link
Contributor

Ah I didnt know they showed up in return positions! For that we probably have no choice but and enum or custom struct

@fitzgen
Copy link
Member Author

fitzgen commented Aug 6, 2018

We could punt in that case and do JsValue too.

@fitzgen
Copy link
Member Author

fitzgen commented Aug 6, 2018

@jonathanKingston,

Supporting document.create_element

Good call! I'll file an issue and make it a blocker.

Exposing document: Document or a simple way to create without using #[wasm_bindgen]

I think exposing pub fn window() -> Window should be good enough, because then one can do window().document().whatever().

And it saves us from having to special case every single global property.

What do y'all think?

Allowing upcasting create_element() result to HtmlDivElement

Yep, this is part of implementing RFC #2.

Provide a simple example of web_sys building up a form or table and inserting it into body().

Something like the eventual example for #446?

@fitzgen
Copy link
Member Author

fitzgen commented Aug 6, 2018

@Pauan,

If nobody else wants to, I can write up a trait-inheritance RFC.

That would be awesome!

@Pauan
Copy link
Contributor

Pauan commented Aug 8, 2018

@fitzgen Done: rustwasm/rfcs#3

@fitzgen
Copy link
Member Author

fitzgen commented Aug 8, 2018

Incredible! Thanks @Pauan !

@jonathanKingston
Copy link
Contributor

It would be nice if there were a way to clone the Element interfaces or the methods accept references instead.
Without this it seems like Rust would have to query the DOM again to get a reference to an element after appending it to the DOM.

And it saves us from having to special case every single global property.
What do y'all think?

Yeah this is fine, I forgot about window for some reason that day.

Yep, this is part of implementing RFC #2.

It looks like that is implemented but I can only upcast to Element and not downcast to HTMLDivElement is this not implemented yet or am I missing something?

Something like the eventual example for #446?

Yeah :)

@fitzgen
Copy link
Member Author

fitzgen commented Aug 23, 2018

It looks like that is implemented but I can only upcast to Element and not downcast to HTMLDivElement is this not implemented yet or am I missing something?

You can use JsCast::dyn_{into,ref,mut}: https://rustwasm.github.io/wasm-bindgen/api/wasm_bindgen/trait.JsCast.html#method.dyn_into

@fitzgen
Copy link
Member Author

fitzgen commented Aug 30, 2018

Also added #257 as a blocker -- not being able to register event listeners and all that is a deal breaker.

@fitzgen
Copy link
Member Author

fitzgen commented Aug 30, 2018

@alexcrichton and I also talked about web-sys compilation times, and whether getting them reasonably fast should be a blocker for publishing.

One idea was to add a cargo feature for every interface, and only generate code for the enabled interfaces.

We should decide (1) if compilation times should be a blocker (and what kind of time is Good Enough to unblock), and (2) if so, what strategy to pursue.

@fitzgen
Copy link
Member Author

fitzgen commented Aug 30, 2018

If we do the feature-per-interface thing, then we need to ensure that docs are generated with everything enabled.

Ideally, each method's doc comment would also list the features required to enable it.

@fitzgen
Copy link
Member Author

fitzgen commented Aug 30, 2018

If we did the interface filtering in the compiler choosing what backend::ast to create, rather than in the build.rs choosing which files to include in the input, then I think we could more easily add docs describing features necessary for enabling a method.

@alexcrichton
Copy link
Contributor

I've posted #790 to help bring compile times under control for web-sys

@fitzgen
Copy link
Member Author

fitzgen commented Sep 12, 2018

Just a note: we probably want to publish a blog post when at the same time that we publish web-sys.

@fitzgen
Copy link
Member Author

fitzgen commented Sep 13, 2018

A couple final things we talked about in todays WG meeting:

  • Do a final audit of unsupported warnings and determine if any of them are blocking critical web APIs (eg the body for fetch thing in web-sys: Unable to add body to a Request #817)

  • Do an audit for moz-prefixed APIs that shouldn't be included in web-sys

@chinedufn
Copy link
Contributor

chinedufn commented Sep 20, 2018

@fitzgen how does web-sys not being announced with Rust 2018 change the blockers for publishing?

i.e. does it need to be as polished before it starts getting published?

Full transparency mostly asking because I've been running into issues with depending on git URLs and things either not compiling (mostly do to me misunderstanding things) or changing under my feet as new code gets pushed every day haha.

Not trying to say that publishing now is the best thing to do, but more so just trying to understand what the new timetable is and if the checklist should still be the same.

Thank you so much!!

@fitzgen
Copy link
Member Author

fitzgen commented Sep 25, 2018

@chinedufn nothing has changed, we're on the same schedule and have the same goals we've always had. Looking to publish this week!

Understand the pain git deps, but very much appreciate your beta testing ❤️ The hassle will be over soon!

@fitzgen
Copy link
Member Author

fitzgen commented Sep 25, 2018

Did another audit of unsupported logs, and there are almost none left:

      7 Unsupported WebIDL Stringifier interface member
      6 Unsupported WebIDL iterable interface member
      4 Unsupported WebIDL Maplike interface member
      2 Unsupported webidl stringifier

@alexcrichton
Copy link
Contributor

To expand on that list:

  • 7 "stringifier" operations, which according to WebIDL means there's a non-default conversion to a string. None of the stringifiers in our WebIDL have function names, though, so these can all likely be safely ignored. We don't currently have a js-to-string builtin but we could certainly add one! (and these would simply inherit such behavior)
  • 6 iterable operations - FormData, NodeList, DOMTokenList, URLSearchParams, Headers, MediaKeyStatusMap. This is covered by Add iterable support for webidl #514
  • 3 maplike members - mostly around some seemingly esoteric APIs, easy to add support for later

In that sense we're definitely looking quite good! I don't think we're missing out on any crucial functionality with the above missing apis

@fitzgen
Copy link
Member Author

fitzgen commented Sep 26, 2018

And we're done here ;)

https://rustwasm.github.io/2018/09/26/announcing-web-sys.html

@fitzgen fitzgen closed this as completed Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen web-sys Issues related to the `web-sys` crate
Projects
None yet
Development

No branches or pull requests

8 participants