-
Notifications
You must be signed in to change notification settings - Fork 62
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
Comments
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. |
Oh no I already have a working branch. It works! 😁 win win Maybe I expressed myself incorrectly sorry. This thing 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.
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.
Oh that's why the wasm binary is bloated without wasm-opt? Good to know, thanks! |
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:
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 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.
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. |
Thanks... but I actually know all of that 😅 I'm one of the maintainer of xtask-wasm! 😁 But thanks for all the information. |
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 |
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 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 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. |
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
Right now you are using dominator, but if you choose to start using other crates, those crates might use
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.
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 |
The bundler will produce:
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.)
I don't use other crates.
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.
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.
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 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. |
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) |
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 Even if you choose to not use that plugin, the bundler will not produce a separate file for the So even in the worst case scenario you will have 3 files:
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 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.
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 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 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 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.
My concern is more with the use of
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?"
That's not an issue, it just requires it to be wrapped in
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.
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.
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. |
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. |
Oh, also, There is a way to workaround it, so it's not a showstopper. |
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:
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. |
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.
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.
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
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.
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.
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.
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.
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:
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.
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. |
My bad sorry, I meant "minimum amount of files possible".
Lot of things are very simple CLI tool. That really does not say how you qualify wasm-pack.
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.
That's a nice analogy.
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 understand it started from the best intention but clearly the result is not what you intended.
I have a real app who does nothing of the sort at all 😂 |
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.
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).
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 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. |
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!
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 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)
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. |
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. |
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.
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 |
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.
That's the plan, yeah. |
Version 0.5.27 has removed all usages of inline_js. |
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.
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] |
If you are unable to answer (for example because of an NDA) you can just say so.
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.
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. |
....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) |
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. |
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:
This is what I came up with to get rid of
set_property
for now: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.
The text was updated successfully, but these errors were encountered: