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

Create the web-sys crate mechanically from WebIDL #409

Merged
merged 42 commits into from
Jul 9, 2018

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Jul 6, 2018

I vendored all the WebIDL from mozilla-central and then used an apache-style available/enabled symlinking thing to choose which ones we actually generate bindings for. Right now the only thing we actually generate bindings for is Event.

See the changes to the guide for details on the expected workflow for adding support for more Web APIs.

This is still WIP because I'm running into issues getting the project builder working for different crates. The way it wants to link package.json and friends is a bit funky. Nothing too hard to fix, but I am not going to come back to this for a couple days and figured folks might want to provide some early feedback.

fitzgen added 14 commits July 6, 2018 09:17
This will eventually contain all the WebIDL-generated bindings to Web APIs.
This is necessary for the WebIDL frontend, which can't translate many WebIDL
constructs into equivalent wasm-bindgen AST things yet. It lets us make
incremental progress: we can generate bindings to methods we can support right
now even though there might be methods on the same interface that we can't
support yet.
It was only `pub` so that we could test it, but we ended up moving towards
integration tests rather than unit tests that assert particular ASTs are parsed
from WebIDL files.
Instead of going into an infinite loop, detect when webpack-dev-server fails to
start up and early exit the test.
This helps for debugging failing WebIDL-related tests.
Still need to fix and finish the test.
@fitzgen fitzgen added the frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen label Jul 6, 2018
Copy link
Contributor

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic to me!

.travis.yml Outdated
install:
- npm install
script:
- (cd crates/web-sys && cargo test)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally also a fan of cargo test --manifest-path crates/web-sys/Cargo.toml

That way if we don't fill it in the error message doesn't look quite so bizarre
Otherwise we don't see any output!
Make sure these are looked up in the git project root rather than the crate root
This was meant for debugging and is otherwise pretty noisy
These are the WebIDL interfaces that we will actually generate bindings for (or
at least bindings for *some* of the things defined in these files).

These are all symlinks into the `webidls/available` directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that windows doesn't support symlinks very well, could we instead perhaps move files from available to enabled?

@fitzgen
Copy link
Member Author

fitzgen commented Jul 9, 2018

Looks like appveyor headless testing is failing with this now:

errors:
    exception: __wbg_f_values_values_Array_target is undefined
    stack: __wbg_f_values_values_Array@http://localhost:8080/1.bundle.js:695:5
    __wbg_f_values_values_Array@http://localhost:8080/bundle.js:59:23
    wasm_bindgen::js::Array::values::h31b91cbdcb83beef@http://localhost:8080/bundle.js:2676:1
    test80::get_values::h95694f03c9da97e7@http://localhost:8080/bundle.js:1544:1
    get_values@http://localhost:8080/bundle.js:1740:1
    get_values@http://localhost:8080/1.bundle.js:312:27
    test@http://localhost:8080/0.bundle.js:22:36
    run@http://localhost:8080/bundle.js:9489:23
    ./run.js/

@fitzgen
Copy link
Member Author

fitzgen commented Jul 9, 2018

Looks like appveyor headless testing is failing with this now:

I think this is failing on master as well.

@fitzgen fitzgen changed the title [WIP] Create the web-sys crate mechanically from WebIDL Create the web-sys crate mechanically from WebIDL Jul 9, 2018
We only use it to serialize headless tests so that we don't try to bind the port
concurrently. Its OK to run another headless test if an earlier one panicked.
Allows dynamically casting values to `js::Object` and `js::Function`.
@alexcrichton
Copy link
Contributor

Bah that's probably myself botching a merge, sorry about that! Looks like AppVeyor is running Firefox 59 but Array.prototype.values was added in Firefox 60, which'd explain the failing test.

If we create bindings to a method that doesn't exist in this implementation,
then it shouldn't fail until if/when we actually try and invoke that missing
method.
src/js.rs Outdated
@@ -443,6 +443,16 @@ extern "C" {
pub fn to_string(this: &Function) -> JsString;
}

impl JsValue {
pub fn as_function(&self) -> Option<Function> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this perhaps be &Function via a transmute? (same w/ as_object below)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On it.

@fitzgen
Copy link
Member Author

fitzgen commented Jul 9, 2018

Ok, finally got CI green! Going to merge!

@fitzgen fitzgen merged commit f2f2d72 into rustwasm:master Jul 9, 2018
alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this pull request Oct 11, 2018
This commit fixes instantiation of the wasm module even if some of the
improted APIs don't exist. This extends the functionality initially
added in rustwasm#409 to attempt to gracefully allow importing values from the
environment which don't actually exist in all contexts. In addition to
nonexistent methods being handled now entire nonexistent types are now
also handled.

I suspect that eventually we'll add a CLI flag to `wasm-bindgen` to say
"I assert everything exists, don't check it" to trim out the extra JS
glue generated here. In the meantime though this'll pave the way for a
wasm-bindgen shim to be instantiated in both a web worker and the main
thread, while using DOM-like APIs only on the main thread.
@grovesNL grovesNL mentioned this pull request Dec 4, 2019
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants