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

Getting rid of inlined JS #68

Closed
cecton opened this issue Jun 29, 2022 · 26 comments
Closed

Getting rid of inlined JS #68

cecton opened this issue Jun 29, 2022 · 26 comments

Comments

@cecton
Copy link

cecton commented Jun 29, 2022

Hello,

I'm looking at getting rid of the inlined JS because I need to have the minimum file possible for a project.

Here is the relevant part of dominator's code:

#[wasm_bindgen(inline_js = "
    export function set_property(obj, name, value) { obj[name] = value; }

    export function add_event(elem, name, capture, passive, f) {
        elem.addEventListener(name, f, {
            capture,
            passive,
            once: false,
        });
    }

    export function add_event_once(elem, name, f) {
        elem.addEventListener(name, f, {
            capture: true,
            passive: true,
            once: true,
        });
    }

    export function remove_event(elem, name, capture, f) {
        elem.removeEventListener(name, f, capture);
    }
")]
extern "C" {
    // TODO move this into wasm-bindgen or gloo or something
    // TODO maybe use Object for obj ?
    pub(crate) fn set_property(obj: &JsValue, name: &str, value: &JsValue);

    // TODO replace with gloo-events
    pub(crate) fn add_event(elem: &EventTarget, name: &str, capture: bool, passive: bool, f: &Function);
    pub(crate) fn add_event_once(elem: &EventTarget, name: &str, f: &Function);
    pub(crate) fn remove_event(elem: &EventTarget, name: &str, capture: bool, f: &Function);
}

This is what I came up with to get rid of set_property for now:

use wasm_bindgen::prelude::*;

#[cfg(feature = "wee_alloc")]
#[global_allocator]
static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT;

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_namespace = console)]
    fn log(message: &str);
    #[wasm_bindgen(js_namespace = Object, js_name = defineProperty)]
    fn define_property(obj: &JsValue, prop: &str, descriptor: &JsValue);
    #[wasm_bindgen(js_namespace = Object, js_name = fromEntries)]
    fn from_entries(iterable: &JsValue) -> JsValue;
    #[wasm_bindgen(js_namespace = Object, js_name = assign)]
    fn assign(target: &JsValue, source: &JsValue);
    #[wasm_bindgen(js_namespace = Array, js_name = of)]
    fn array_of(of: &JsValue) -> JsValue;
    #[wasm_bindgen(js_namespace = Array, js_name = of)]
    fn array_of_2(of_1: &JsValue, of_2: &JsValue) -> JsValue;
    #[wasm_bindgen(js_namespace = Array, js_name = of)]
    fn array_prop_key_value(key: &str, value: &JsValue) -> JsValue;
}

#[wasm_bindgen(start)]
pub fn run_app() -> Result<(), JsValue> {
    #[cfg(feature = "console_error_panic_hook")]
    console_error_panic_hook::set_once();

    // Solution 1: using Object.defineProperty and Object.fromEntries
    let descriptor = js_sys::Array::new();
    descriptor.push(&js_sys::Array::from_iter([JsValue::from("value"), JsValue::from(42_i32)]));
    descriptor.push(&js_sys::Array::from_iter([JsValue::from("writable"), JsValue::from(true)]));
    descriptor.push(&js_sys::Array::from_iter([JsValue::from("configurable"), JsValue::from(true)]));
    descriptor.push(&js_sys::Array::from_iter([JsValue::from("enumerable"), JsValue::from(true)]));
    let descriptor = from_entries(&descriptor);
    let obj = js_sys::Object::new();
    define_property(&obj, "foo", &descriptor);
    log(&format!("it works: {:?}", obj));

    // Solution 2: using Object.fromEntries and js_sys::Array::from_iter
    let obj = js_sys::Object::new();
    assign(&obj, &from_entries(&js_sys::Array::from_iter([&js_sys::Array::from_iter([JsValue::from("bar"), JsValue::from(42_i32)])])));
    log(&format!("it works: {:?}", obj));

    // Solution 3: using Object.fromEntries and Array.of and js_sys::Array::from_iter
    let obj = js_sys::Object::new();
    assign(&obj, &from_entries(&array_of(&js_sys::Array::from_iter([JsValue::from("baz"), JsValue::from(42_i32)]))));
    log(&format!("it works: {:?}", obj));

    // Solution 4: using Object.fromEntries and Array.of
    let obj = js_sys::Object::new();
    assign(&obj, &from_entries(&array_of(&array_of_2(&JsValue::from("foobar"), &JsValue::from(42_i32)))));
    log(&format!("it works: {:?}", obj));

    // Solution 5: using Object.fromEntries and Array.of and a special Array.of that takes &str in argument
    let obj = js_sys::Object::new();
    assign(&obj, &from_entries(&array_of(&array_prop_key_value(wasm_bindgen::intern("foobaz"), &JsValue::from(42_i32)))));
    log(&format!("it works: {:?}", obj));

    Ok(())
}

Do you think this could be a good way to go or do you have another idea in mind?

Solution 5 looks the more efficient but I have not done benchmarks yet.

@Pauan
Copy link
Owner

Pauan commented Jun 29, 2022

@cecton I'm looking at getting rid of the inlined JS because I need to have the minimum file possible for a project.

If that's your goal, then removing the inline_js won't help.

web-sys also generates the exact same JS wrappers, so the file size will be the same (or even bigger with web-sys).

Similarly, js-sys also generates JS wrappers. There currently is no way to avoid the JS wrappers.

In fact, using inline_js can sometimes reduce file size, because you're able to generate the exact JS code you need.

Using the Rollup plugin will generate the smallest file size. In addition you can follow the steps listed here. The Rollup plugin automatically runs wasm-opt, so you don't need to do that.

Unfortunately there is a bug in rustc which causes it to include a lot of unnecessary debug information, this bloats up the file size but it's unrelated to wasm-bindgen or dominator.

@cecton
Copy link
Author

cecton commented Jun 29, 2022

If that's your goal, then removing the inline_js won't help.

Oh no I already have a working branch. It works! 😁 win win

Maybe I expressed myself incorrectly sorry.

This thing #[wasm_bindgen(inline_js = "...")], more particularly the inline_js, generates a JS file in the snippets directory in output. This is what I'm getting rid of because I don't want more extra files.

So I removed it entirely, used web_sys for the events stuff instead, and replaced the set_property by solution 5. It's working just fine, no more snippet file.

Using the Rollup plugin will generate the smallest file size. In addition you can follow the steps listed here. The Rollup plugin automatically runs wasm-opt, so you don't need to do that.

I know that already actually. It's unrelated to my issue. My issue is not with the size but with the number of files. I would like to avoid unnecessary extra file. For now I'm using Yew but I'm investigating other frameworks and dominator is a good candidate for my use.

Unfortunately there is a bug in rustc which causes it to include a lot of unnecessary debug information, this bloats up the file size but it's unrelated to wasm-bindgen or dominator.

Oh that's why the wasm binary is bloated without wasm-opt? Good to know, thanks!

@Pauan
Copy link
Owner

Pauan commented Jun 29, 2022

This is what I'm getting rid of because I don't want more extra files.

Well in that case you should be using a bundler like Rollup (ideally) or Webpack. Users aren't supposed to be running wasm-bindgen manually, it's intended to always use a bundler. A bundler does a lot of useful things:

  • It automatically installs the correct version of wasm-bindgen-cli (which matches the version of wasm-bindgen in Cargo.lock). This is super important, otherwise you will get lots of bugs and problems.
  • It automatically installs and runs wasm-opt.
  • It bundles multiple files together into a single file.
  • It can bundle other assets too (like CSS, JSON, etc.)
  • It automatically optimizes and minifies all of your code, making your code smaller and faster.
    wasm-bindgen doesn't bother to optimize the JS code, because it's expected that the bundler will do the optimization.
    So if you want the smallest and fastest code, using a bundler is mandatory.
  • It makes it very easy to call Rust functions from JS (and vice versa).
  • It makes it easy to use npm packages (and it will combine the npm packages into a single optimized file).
  • It makes the dev experience much better, because it supports file watching, auto-rebuild, and livereload. You no longer need to manually recompile your code. This saves a lot of time.

There is very little reason to not use a bundler. A bundler makes your code faster, smaller, and saves you a lot of time and stress. It is standard practice to use a bundler when doing anything JS or web related.

And wasm-bindgen was intentionally designed to integrate with bundlers, because wasm-bindgen isn't trying to replace the JS ecosystem, it's trying to work together with the JS ecosystem.

All of the dominator examples use Rollup, so if you don't understand how to use bundlers, that's not a problem! Just copy one of the examples, run yarn install, and then yarn start. It will open up the webpage in your browser, and it will automatically recompile whenever you make any changes.

You'll need to install node. And you can download yarn from here. If you're using Linux, your distro should already have node and yarn available in your package manager.

Oh that's why the wasm binary is bloated without wasm-opt? Good to know, thanks!

Yes, wasm-bindgen is a low-level tool, it's not supposed to be used by users. Instead it's supposed to be used by higher-level tools like Rollup / Webpack / wasm-pack.

If you use wasm-bindgen manually, then you'll need to do a lot of extra steps yourself. But it's not worth it, using a bundler like Rollup is much easier and gives better results.

@cecton
Copy link
Author

cecton commented Jun 29, 2022

Thanks... but I actually know all of that 😅 I'm one of the maintainer of xtask-wasm! 😁 But thanks for all the information.

@cecton
Copy link
Author

cecton commented Jun 29, 2022

I still need no extra file for reasons unrelated to bundling itself. As I said, I need minimum of files on a project and the use of this feature generates one extra file which can be avoided.

@Pauan
Copy link
Owner

Pauan commented Jun 29, 2022

I still need no extra file for reasons unrelated to bundling itself. As I said, I need minimum of files on a project and the use of this feature generates one extra file which can be avoided.

Why is that necessary? Since inline_js is an incredibly useful and often necessary tool, I think it's unreasonable to expect every Rust crate to not use it.

@cecton
Copy link
Author

cecton commented Jun 29, 2022

Why is that necessary?

I'm sorry to repeat my self: as I said, I need minimum of files on a project. The project is private and I don't want to get into details but I have technical limitations that makes it complicated for me to serve many files.

I think it's unreasonable to expect every Rust crate to not use it.

I totally agree with you there... but I never said that I expect every Rust crate to not use it. I don't even know how you got that message from what I said. I'm not in a crusade against evil inline js 😓

Here is my reasoning: rust-dominator is a web framework comparable to yew. Yew does not require the inline_js feature. dominator does but I proved it's possible to do without it for probably 0 cost, so it's probably not super useful. If it doesn't hurt and it's easy to try, let's try it and see how it goes. Why not? My first PR is already reducing the number of lines of code and I didn't hear any comment against its implementation.

What are you fighting against here exactly? Do you have arguments to keep inline_js or against the implementation? Do you want to propose an alternative?

It's your project, you do what you want with it but I'll be gone if I can't use it. It's a crude way to say it but accurate.

@Pauan
Copy link
Owner

Pauan commented Jun 29, 2022

I'm sorry to repeat my self: as I said, I need minimum of files on a project.

Yes, but that doesn't explain why. If you need to serve a minimum of files, then the logical answer is to use a bundler. So I'm very curious why you are in a situation where even 1 extra file is a big problem, but you are unable to use a bundler. I'm having a hard time imagining a scenario like that.

In fact, if you use the Rollup plugin, then you can use inlineWasm: true, which will inline the .wasm file into the .js file, so that's even fewer files. Ideally you will end up with just 1 .js file and nothing else, not even a .wasm file.

I don't even know how you got that message from what I said. I'm not in a crusade against evil inline js

Right now you are using dominator, but if you choose to start using other crates, those crates might use inline_js. So if even 1 extra file is too much, then you would have only 3 choices:

  1. Don't use those crates.
  2. Convince those crates to not use inline_js.
  3. Use a bundler.

Since you opted for option 2 for dominator (instead of option 3), I imagine that you would also use option 2 for those other crates as well. It's not a very big leap of logic.

What are you fighting against here exactly? Do you have arguments to keep inline_js or against the implementation? Do you want to propose an alternative?

I'm just trying to understand your use case. Most of the time when people have issues, they come up with a solution for it, but there is an even better potential solution. I'm very conscious of the XY problem, and so I always ask for people to explain their use case, even if I agree with their proposal. The reasons why people want to do things usually matter more than the how. This habit of mine has often resulted in much better solutions.

There are some arguments to use inline_js (slightly smaller file size and faster performance), but they are minimal, and can be fixed. In the TODO notes I even mention that I plan to replace the inline_js with gloo-events. From a technical standpoint I don't have any major objections.

@cecton
Copy link
Author

cecton commented Jun 29, 2022

If you need to serve a minimum of files, then the logical answer is to use a bundler.

The bundler will produce:

  • index.html: required
  • app.js: required (but I embed it in the index.html)
  • app.wasm: required
  • optional snippets files: dominator has one snippet file because of the use of inline_js

In fact, if you use the Rollup plugin, then you can use inlineWasm: true, which will inline the .wasm file into the .js file, so that's even fewer files. Ideally you will end up with just 1 .js file and nothing else, not even a .wasm file.

I didn't know this is nice! Though would that hurt the size of the file if the binary is encoded as a JS byte sequence? (I do have limitations on size too.)

Right now you are using dominator, but if you choose to start using other crates, those crates might use inline_js.

I don't use other crates.

Since you opted for option 2 for dominator (instead of option 3)

I minimized the bundle as much as I could already (2 files). There is nothing I can do to prevent that snippet js file to be created by wasmbindgen. My only option is to get rid of the inlined JS or read it from disk and embed it in the index.html like I did already for the app.js. I would definitely embed more things in the index.html if necessary but it does look like dominator doesn't really need it as it can easily be avoided.

slightly smaller file size and faster performance

It sounds like you are sure of this but I personally find those arguments doubtful.

Smaller file size: I'm literally moving a functionality that is in a JS file to the WASM. Yes the wasm will get slightly bigger but the JS file will get smaller. It's more likely that the binary format of the wasm will also be less wasteful than the JS source file.

Faster performance: right now dominator does call set_property with a &str already. All those events method from web_sys are just wrappers really, just like what is in dominator at the moment. On top of that you do say yourself you would want to use gloo-events so it seems to me you actually are okay to lose a bit of performance here. But you use this argument against using web-sys which seems contradictory.

Not that I mind writing some benchmark to see how everything goes though. Even for the file size.

In the TODO notes I even mention that I plan to replace the inline_js with gloo-events.

There are also comments explaining how this is not possible to do because gloo-events lack some functionalities (discard) that are in the current implementation in dominator. I did explore gloo-events to see if it was a good fit but according to the comments I would first need to make PR on gloo-events to integrate the missing functionalities.

The other reason why I did not go any further in this is because web-sys is already in the dependencies while gloo isn't. There is no point of adding a dependency for such a small functionality. If you do want to use gloo you would probably need to replace web-sys entirely by gloo.

My proposal aims to provide minimum impact. Keeping things as they are while allowing me to continue my tests with dominator.

I'm very conscious of the XY problem, and so I always ask for people to explain their use case, even if I agree with their proposal.

I guess this sounds wise but from my perspective it seems we are debating on whether or not implement a small change. I really don't mind if dominator decides to go for gloo later on and remove the things I did or find a better way to set properties (or avoid setting properties). As I said, my idea is to make a minimal impact proposal that brings no downside at all (or nearly) and solve my issue.

@cecton
Copy link
Author

cecton commented Jun 29, 2022

Maybe the best way to end this discussion is to do some benchmark, check the size of the delivered files, and see. (I will provide this since I'm the one coming with the chance ofc)

@Pauan
Copy link
Owner

Pauan commented Jun 29, 2022

Actually, index.html isn't produced by the bundler, that's entirely up to you. Of course you can use a plugin to generate the index.html file, but that's also up to you.

There are also plugins that will inline the .js code into the .html file. By using that plugin and inlineWasm: true you would end up with just 1 index.html file, no .js or .wasm files at all.

Even if you choose to not use that plugin, the bundler will not produce a separate file for the inline_js. The entire point of a bundler is that it bundles all of your .js files together into a single .js file. That's why it's called a "bundler".

So even in the worst case scenario you will have 3 files: index.html, app.js, and app.wasm. And no matter how many inline_js there are, it will never increase the number of files.

Though would that hurt the size of the file if the binary is encoded as a JS byte sequence?

Yes, it increases the file size by approximately 33%.

If you care about file size, then using a bundler is essentially mandatory. In addition to bundling all of your .js files together, it also optimizes the JS code, it does dead code elimination, and it minifies the JS code. All of this significantly reduces the size of your JS code.

Countless thousands of hours have gone into the creation of bundlers, they are very good at what they do, which is why they are universally used within the JS ecosystem. Choosing to not use a bundler is like choosing to use Rust without cargo. It's possible, but certainly an odd choice.

Smaller file size: I'm literally moving a functionality that is in a JS file to the WASM.

That's not correct. As I explained earlier, wasm-bindgen creates JS snippets for every JS API. If you choose to use a js-sys or web-sys API, it will generate a JS snippet. That JS snippet will be as big (or bigger) than the inline_js snippet.

There is no such thing as "moving a JS API into Wasm". All of the APIs are JS APIs, they all use a JS snippet. There is no difference between an inline_js API and a normal #[wasm_bindgen] API, except that inline_js can produce smaller and faster code.

In the future (likely several years in the future) it will be possible for Wasm to bind directly to web APIs without a JS snippet, but that's definitely not possible today.

You claim that the Wasm file will get bigger and the JS file smaller, but that's not how it works. Instead both the Wasm and JS files will stay the same size (plus or minus a very small amount).

If the code does get smaller, it will be because of shared code: if there is another crate which is using web-sys event listeners, or js-sys Reflect::set then it will be able to share the JS snippets for that, which will reduce the code size. But if you're not using other crates, then there should be very little difference.

I used to be on the Rust Wasm Core team, I've collaborated with the Wasm working group, I've contributed heavily to wasm-bindgen (and stdweb), and I created the Rollup plugin. I also completely rewrote the code generation for web-sys from scratch. So I have a quite good idea of what sort of code is generated by wasm-bindgen.

If you disagree, that's fine, I'm not perfect, you can test it out yourself.

right now dominator does call set_property with a &str already

My concern is more with the use of Reflect.set, which I believe is slower than a plain object assignment. There is also a (very) tiny performance cost from converting the &str into a JsValue. But prop isn't used that much, so I don't think it's a big deal either way.

But you use this argument against using web-sys which seems contradictory.

The argument applies equally, because gloo-events (written by me) internally uses web-sys APIs. As I have said repeatedly, on a technical level I am okay with the changes, I am just really trying to understand why you cannot use a bundler (which would solve this problem, and also many other problems). Bundlers are amazingly useful, they have so many benefits. Benefits that are hard to achieve without a bundler.

It's like as if you came up to me and asked, "how can I compile dominator without rustc, without cargo, and without wasm-bindgen?" Obviously the first thing I'm going to ask is "why?"

There are also comments explaining how this is not possible to do because gloo-events lack some functionalities (discard) that are in the current implementation in dominator.

That's not an issue, it just requires it to be wrapped in ManuallyDrop. This does leak a small amount of memory, but I don't think it's a very big deal. We already use ManuallyDrop for ValueDiscard.

The other reason why I did not go any further in this is because web-sys is already in the dependencies while gloo isn't.

That's not a problem at all. Whether the code is in dominator or gloo-events makes no difference, it's the same code either way. And having it be in gloo-events is better because then the code can be shared between multiple crates.

If you do want to use gloo you would probably need to replace web-sys entirely by gloo.

No, why would I need to do that? The entire point of the gloo crates is that they implement one small bit of functionality (by creating a higher-level wrapper around web-sys). You're supposed to be able to cherry pick and use only what you need. And of course combining it with web-sys is not a problem at all.

I guess this sounds wise but from my perspective it seems we are debating on whether or not implement a small change.

The purpose is solely for your benefit. The point of the XY problem is that it causes problems for the person asking the question. And recognizing the XY problem means to help the asker.

If I didn't care about helping you (or other people) then I wouldn't care about the XY problem at all. I would just allow other people to waste time and make horrible suboptimal decisions.

@Pauan
Copy link
Owner

Pauan commented Jun 30, 2022

Maybe the best way to end this discussion is to do some benchmark, check the size of the delivered files, and see. (I will provide this since I'm the one coming with the chance ofc)

If you're going to do that, you should make the comparison using the Rollup plugin, to make sure that the code is as optimized as possible, so we're comparing apples-to-apples. Otherwise you will end up comparing unoptimized code to unoptimized code.

@Pauan
Copy link
Owner

Pauan commented Jun 30, 2022

Oh, also, AddEventListenerOptions doesn't use interning, which is actually a pretty big deal. String interning is critical to performance with Wasm. So maybe the web-sys code generation should be changed so that it interns the key.

There is a way to workaround it, so it's not a showstopper.

@cecton
Copy link
Author

cecton commented Jun 30, 2022

Ok I got it...

TL;DR

I just tested with rollup and it does not create the snippets directory. It's embedded in a single JS file. ✔️ So you were right.

Fun fact: set_property and add_event_once got stripped from the resulting JS too (I ran the "counter" example).

Long story:

I think you confused me with all those things you said:

  • If that's your goal, then removing the inline_js won't help.
    Yes it helps. But using rollup can achieve the same effect. It all depends on how you build the project.
  • Similarly, js-sys also generates JS wrappers. There currently is no way to avoid the JS wrappers.
    I should not have ignored this because it was important. I didn't get what you meant because js-sys and web-sys do no generate extraneous JS files in the snippets directory.
  • In fact, using inline_js can sometimes reduce file size
    I never asked to reduce file size. That felt like you missed the point.
  • Users aren't supposed to be running wasm-bindgen manually, it's intended to always use a bundler.
    You assumed I was not using a bundler for some reason.
  • bundler
    I'm not even sure if we define as bundler the same thing. I understand you know the wasmbindgen stack pretty well so you also know that wasm-pack and trunk are using wasmbindgen. To me it seems wasm-pack and wasmbindgen are bundler. But I suspect by "bundler" you explicitly meant JS bundlers like rollup and webpack.
  • [me: I still need no extra file] Why is that necessary? Since inline_js is an incredibly useful and often necessary tool
    You make it sounds like I need inline_js and you're still questioning my motives. By that you're sending me the message that I should no try to get the minimum file as possible and you failed to accept this as a premise.
  • but if you choose to start using other crates
    For which I answered "I don't use other crates". This is wrong on my side. I meant "other crates that use js snippets". I actually use a lot of other crates like, idk... serde, ws_stream_wasm, futures-timer, instant, once_cell, bytes, chrono, regex, ... none of them required inline js. This is the first time I had encounter the need of using those snippet JS. The last time I saw it was in material-yew which binds to the Material Web Components API which is in TS. So it makes sense that you need to communicate with that API. But it's not like dominator needs those snippet JS, it's clearly doable without it.

The thing that made me "clicked" is when you said: "Even if you choose to not use that plugin, the bundler will not produce a separate file for the inline_js". Because in my mind this was wrong because wasm-pack and everything that relies on wasmbindgen for their backend won't embed the js in one file. So I guess I was missing something. Then you added: "If you're going to do that, you should make the comparison using the Rollup plugin". So now it became clear to me that what I missed is that rollup/webpack bundlers are not the same than wasm-pack/trunk/xtask-wasm bundlers.

I'm also actually teaching people to code and I know the main difficulty is to explain things that are so obvious to us that we fail to explain them properly. Believe it or not I did work in web frontend development with TS and setup a few projects with webpack but I never actually bothered understanding what the bundlers do internally as I was too busy making the web app itself.

In Rust, wasm-pack is sold like the default option but the whole process feels intricate: mixing JS tools with Rust tools. It feels like we shouldn't need to use JS tools (and install nodejs) to make a Rust app, it's counterintuitive. Also because the wasm is already optimized by wasm-opt (which is ran by wasm-pack) so the only missing part is the JS bundling/optimization which is the smallest and most insignificant part in term of size in bytes in my project because I only use JS to bootstrap the wasm app and I embed it manually in the index.html. Sounds like I would need to introduce a certain amount of tools and complexity just to get a single JS file...

Well, anyway, thank you for your time and all your explanation.

@cecton cecton closed this as completed Jun 30, 2022
@Pauan
Copy link
Owner

Pauan commented Jun 30, 2022

Fun fact: set_property and add_event_once got stripped from the resulting JS too (I ran the "counter" example).

Yes, like I said bundlers do a lot of optimizations on JS code. That includes inlining, function hoisting, constant propagation, dead code elimination, variable renaming, minification, all sorts of things.

Just like Rust uses LLVM to optimize Rust code, it is standard practice to use bundlers to optimize JS code. Anybody who cares about file size should be using a bundler.

I never asked to reduce file size. That felt like you missed the point.

When you said "minimum file possible", I interpreted that as meaning file size, since that's by far the most common thing that people complain about with Wasm.

Though in this case it doesn't matter, since either way the solution is to use a bundler, since a bundler produces the smallest number of files and the smallest file size.

I didn't get what you meant because js-sys and web-sys do no generate extraneous JS files in the snippets directory.

Yes, it is true that wasm-bindgen inlines the JS snippets into the JS glue code, but the snippets are still there.

Actually, there was some discussion a few years ago about wasm-bindgen inlining the inline_js into the JS glue code as well, but that ended up being too complicated, which is why wasm-bindgen creates a separate file instead.

You assumed I was not using a bundler for some reason.

Yes, of course, because bundlers always combine multiple JS files into 1 file. So the fact that you had a separate inline_js snippet file proves that you aren't using a bundler. It wasn't an assumption, it was a fact.

I'm not even sure if we define as bundler the same thing. I understand you know the wasmbindgen stack pretty well so you also know that wasm-pack and trunk are using wasmbindgen. To me it seems wasm-pack and wasmbindgen are bundler.

wasm-bindgen is a compiler, wasm-pack is a very simple CLI tool. Neither of them are bundlers, which is why they generate multiple unnecessary JS files. A bundler wouldn't do that.

Bundlers do tend to be JS-specific, because in the JS-world there is a need to combine several JS files into 1 JS file, but that need doesn't really exist anywhere else. Most languages just rely on dynamic linking (or in the case of Rust, static linking).

I think it's reasonable to say that a static linker is a bundler, but nobody calls them bundlers, because the term "bundler" was invented by the JS community. Yes, all of the terminology is confusing for sure, especially for a beginner.

Trunk is an interesting case... it does bundle assets, so it is technically a bundler. But it doesn't bundle multiple JS files together, so it's not a typical JS bundler. And it doesn't do any optimization of JS code. It's a very simple bundler that is missing a lot of features that other bundlers have.

That's why I recommend to use Rollup, because it is a full-featured bundler. Hopefully in the future Trunk will be viable for replacing Rollup.

You make it sounds like I need inline_js and you're still questioning my motives. By that you're sending me the message that I should no try to get the minimum file as possible and you failed to accept this as a premise.

I'm not sure why you're interpreting it like that, all of my advice was about helping you to get the smallest possible number of files, and the smallest possible file size. My advice would achieve your goals without requiring any changes to dominator.

As I repeatedly said, I asked you those questions in order to avoid the XY problem. I asked those questions specifically because I'm taking your goals seriously. If I didn't take your goals seriously, then I would have treated you very differently.

You assumed that the only way to fix your problem is to remove inline_js, but there are alternative (possibly better) solutions. That's why I ask for other people's motives, so I can help them find the best possible solution to their problem.

Even if I removed inline_js from dominator, that wouldn't really fix your problem, because your JS code would still be extremely unoptimized. Using a bundler like Rollup will make your JS code much smaller. With the counter example, the JS code goes from 14,914 bytes (unoptimized) to 7,044 bytes (optimized). That's a 53% reduction in file size.

And that's just for a tiny hello world app. In a real app (like tab-organizer), the unoptimized JS code is 109,442 bytes and the optimized JS code is 52,647 bytes (52% reduction in size). Bundlers make a huge difference. It's like the difference between compiling in debug mode and release mode.

So now it became clear to me that what I missed is that rollup/webpack bundlers are not the same than wasm-pack/trunk/xtask-wasm bundlers.

That's correct, the term "bundler" refers almost exclusively to Rollup / Webpack / Snowpack / Vite / Parcel / etc. because the term "bundler" was invented by the JS community, in order to solve a problem that is specific to JS. I've actually never heard the term "bundler" used outside of the JS community.

In Rust, wasm-pack is sold like the default option but the whole process feels intricate: mixing JS tools with Rust tools. It feels like we shouldn't need to use JS tools (and install nodejs) to make a Rust app, it's counterintuitive.

Yes, there is a lot of dark history with wasm-pack, originally wasm-pack was intended to have a lot more features, but... things happened. Nowadays wasm-pack is almost useless, and so I recommend everybody to use the Rollup plugin instead.

As for mixing JS tools with Rust tools, that was an intentional choice of wasm-bindgen. Yes it is possible to generate JS code without JS tools (that's what wasm-bindgen does), but that's not good enough. Real apps need to:

  • Produce optimized JS code with the smallest number of files and smallest file size.
  • Interoperate with existing JS code.
  • Use npm packages.
  • Publish Rust code to npm.

The existing JS tools handle all of that very well, countless thousands of hours have gone into making those tools. The Rust Wasm team is small, there's no way we could create a Rust tool that replicates what the JS tools already do.

And so the choice is obvious: use JS tools for handling JS code, and Rust tools to handle Rust code. That way we get the optimum result with the least amount of effort.

Sounds like I would need to introduce a certain amount of tools and complexity just to get a single JS file...

Yes, which is why I worked very hard to make the Rollup plugin as easy to use as possible.

Previously the recommended solution was to use Webpack, but Webpack is very complex. Using Rollup is much much simpler.

@cecton
Copy link
Author

cecton commented Jul 1, 2022

When you said "minimum file possible", I interpreted that as meaning file size, since that's by far the most common thing that people complain about with Wasm.

My bad sorry, I meant "minimum amount of files possible".

wasm-pack is a very simple CLI tool.

Lot of things are very simple CLI tool. That really does not say how you qualify wasm-pack.

It wasn't an assumption, it was a fact.

imo trunk and xtask-wasm are bundlers and they do not do all the job JS bundlers do. I don't know how you qualify wasm-pack but for me it's also a bundler. It's a bundler for Rust wasm apps.

I think it's reasonable to say that a static linker is a bundler, but nobody calls them bundlers, because the term "bundler" was invented by the JS community.

That's a nice analogy.

My advice would achieve your goals without requiring any changes to dominator.

tbh... I excluded dominator from my list when I closed this ticket.

As I said, if I want to use dominator I would be forced to add rollup and nodejs to the process. I'm not going to complexify things because dominator use a few inlined JS it could write differently. This is a Rust project, the size of the JS won't change at all, only the size of the WASM will grow, so I don't really care about super optimizing the JS, it's just a bootstrap.

I asked you those questions in order to avoid the XY problem.

I understand it started from the best intention but clearly the result is not what you intended.

Real apps need to:

  • Produce optimized JS code with the smallest number of files and smallest file size.
  • Interoperate with existing JS code.
  • Use npm packages.
  • Publish Rust code to npm.

I have a real app who does nothing of the sort at all 😂

@Pauan
Copy link
Owner

Pauan commented Jul 2, 2022

Lot of things are very simple CLI tool. That really does not say how you qualify wasm-pack.

wasm-pack is just a tiny wrapper around wasm-bindgen, it barely does anything itself. It doesn't do any bundling whatsoever, all of its functionality is provided by wasm-bindgen.

tbh... I excluded dominator from my list when I closed this ticket.

That's your choice, but it's an odd one since I have repeatedly agreed with removing inline_js (and have had a TODO comment about it even before you made the PR).

This is a Rust project, the size of the JS won't change at all, only the size of the WASM will grow, so I don't really care about super optimizing the JS, it's just a bootstrap.

I'm definitely getting extremely mixed signals here. This entire issue has been because you do care very much so about optimizing the JS code, to the point where even 1 extra (tiny) file is too much for you. And you also mentioned earlier that you do care about the JS file size.

I also doubt that the JS won't change, because every time you use a JS API (or use a crate which uses a JS API, like rand / crono / etc.) then the JS will increase. It's nice to have the flexibility to add new crates without having to worry about the performance, because everything is optimized.

Though I do agree with you that the Wasm size will usually dwarf the JS size... the JS size is not insignificant (saving 55 kB makes a big difference on the web).

I understand it started from the best intention but clearly the result is not what you intended.

I can't control your emotions, or your reactions, or your choices. And my intention was always to give you the best advice and knowledge possible to solve your problem. I achieved that goal, regardless of whether you accept my advice or not.

@cecton
Copy link
Author

cecton commented Jul 2, 2022

As your app grows, so is the size of the wasm. The JS might grow a little bit but only in some particular cases.

In my use case, I don't mind the size of the JS (I would if it reached 1MB obviously). I mind the number of files and the wasm size because I have particular limitations.

Having everything in one single index.html is interesting to know. Thanks!

(and have had a TODO comment about it even before you made the PR)

I know. I also found weird that you leave a todo in the code but keep pushing on me using rollup. I won't gain much using rollup in this case.

If you do want to get rid the the inline js for set_property you can pick one of the solutions I proposed or propose another idea.

For the events listener, the PR I made works. But I hear I should look at gloo events instead. I can do that.

(If your intention is to only eliminate part of them, it won't help me because I want to get rid of the extra snippet file)

my intention was always to give you the best advice and knowledge possible to solve your problem

It doesn't matter that you gave the best advice if the one person who needs it won't use it. At the end you still need a discussion to reach an agreement of some sort. Otherwise you did not help them.

@Pauan
Copy link
Owner

Pauan commented Jul 4, 2022

At the end you still need a discussion to reach an agreement of some sort. Otherwise you did not help them.

Which is why I have repeatedly asked you to explain your use case, and motivation, so I can give you the best advice. You have stubbornly refused, so I gave the best advice I could in the situation. And my advice does indeed fix the problem for you (plus it fixes other problems too). Clearly you don't like the advice, yet you still refuse to explain why. There's nothing I can do about that. You are the one who is choosing to not engage in discussion.

@cecton
Copy link
Author

cecton commented Jul 4, 2022

Which is why I have repeatedly asked you to explain your use case, and motivation

I mean... yeah... at some point you should have realized I was not willing to give you more details than what I wanted too.

It's clearly not your place to question what I do. It is great that you want to help but you shouldn't force your advice on someone.

I have repeatedly agreed with removing inline_js (and have had a TODO comment about it even before you made the PR).

So... are we going to get rid of those inlined JS? Right now I expect that you would ask for some perf testing on all the solutions to see what's best. Maybe some tweeks on how to improve speed. Maybe a solution 6 this is better-faster-stronger... Tell me

@Pauan
Copy link
Owner

Pauan commented Jul 6, 2022

I mean... yeah... at some point you should have realized I was not willing to give you more details than what I wanted too.

That's your choice, but you're not going to get very far with that attitude, especially when you're asking favors of other people.

It does help to be humble, and answer other people's questions, it's just common courtesy. Not everybody is as patient as I am.

So... are we going to get rid of those inlined JS?

That's the plan, yeah.

@Pauan
Copy link
Owner

Pauan commented Jul 6, 2022

Version 0.5.27 has removed all usages of inline_js.

@cecton
Copy link
Author

cecton commented Jul 6, 2022

It does help to [...] answer other people's questions, it's just common courtesy.

I don't necessarily have the freedom to do so. Even if I had, imo you are overstepping your role here. I came with a specific issue, I didn't came asking for advice. You forced the advice to me.

Not everybody is as patient as I am.

Not everybody is as patient as I am.

when you're asking favors of other people.

I'm not asking any favor. I'm contributing to an OSS project. If someone is doing a favor here it's me. But that's not how I see it, I see it as sharing time and effort towards a common goal.

[edit: spelling]

@Pauan
Copy link
Owner

Pauan commented Jul 6, 2022

I don't necessarily have the freedom to do so.

If you are unable to answer (for example because of an NDA) you can just say so.

Even if I had, imo you are overstepping your role here. I came with a specific issue, I didn't came asking for advice. You forced the advice to me.

Nobody is forcing anything onto you. What an absurd accusation to make.

You come in and ask for changes, I politely ask for you to explain why you need these changes, you refuse, I give you a large amount of information and advice (which I would normally charge $70 an hour for, my knowledge is very valuable), I make the changes you asked for, and now you start to falsely accuse me of "forcing" things onto you.

You have acted incredibly rude this entire time. You have no respect at all for other people's knowledge or experience, and no respect at all for other people's time. You are one of the most rude and entitled people I have met in years.

I'm not asking any favor. I'm contributing to an OSS project.

Nobody is required to accept any of your changes. You aren't entitled to have your changes accepted. You are not in charge of other people's projects. Asking for a project to accept your change is asking for a favor. It is up to you to convince the maintainer that they should accept your changes, otherwise they won't. You clearly don't understand at all how open source works. And you also don't understand how human social interactions work either.

With your arrogant attitude and lies, quite frankly I don't think I will be accepting any changes from you. I think it would be better if you use a different project.

@cecton
Copy link
Author

cecton commented Jul 6, 2022

....I'm deeply sorry I caused you such distress. I never meant you any harm.

Please forget me entirely and forget this discussion ever happened.

(you should lock the conversation so we stop it for good)

@dakom
Copy link

dakom commented Jul 11, 2022

Unrelated - I have recently run into an issue of needing to remove js snippets for exporting a library to be used in node. It's painful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants