-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove dependency renaming to enable WASM #621
Conversation
Regarding the failing
|
You need to add |
Wow, great work. As kdy already mentioned, yep it seems that you've forgotten to add aliases to the imported names (just like in javascript - you can't just rename imported variable, you should assign the old name as the alias in order for things to keep working, otherwise, to rename all usages, which is more time-consuming) |
It seems like |
Btw, it's a breaking change as wasm module cannot be loaded synchronously, so transformSync and transformFileSync cannot be used. |
I think |
@@ -1,6 +1,6 @@ | |||
use std::path::Path; | |||
use swc::{ | |||
ecmascript::{ | |||
swc_ecmascript::{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be ecmascript
@@ -2,11 +2,11 @@ use swc; | |||
|
|||
use std::{path::Path, sync::Arc}; | |||
use swc::{ | |||
common::{ | |||
config::Options, | |||
swc_common::{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be common
pub use sourcemap; | ||
pub use swc_atoms; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be pub use swc_atoms as atoms
@kdy1 Hi, are you sure? |
Oh.. I didn't know that. Then if wasm's performance is not bad, (Edited as it does not seem like a simple decision) I've searched about it, and it seems like the size of the synchronous wasm module is limited to 4kb. I'll publish it as @swc/wasm |
I don't know if you are currently making and distributing binaries, but for me that's hell (all the ABI-compatibility quirks). Wasm is so much easier, it's breeze. That said, wasm is 2-3 times slower than native (at least for me, on Windows), but still incredibly faster than JS (in some tasks, e.g. processing strings, x30 faster). |
Could you share the link to that info? Seems weird. I am loading a wasm module that's a few Mb (if we talk about .wasm file size) and it works fine. If you do this, how are you going to sync changes and files between the two repos? Is there an easy way I am not aware of? |
@anurbol https://developers.google.com/web/updates/2018/04/loading-wasm |
@kdy1 I guess the limitation is only for browsers. For node.js (where SWC is supposed to be used, i guess) that limitation just doesn't make sense. |
@kdy1 Also, I didn't even touch the WebAssembly API, I am just using Wasm-Bindgen and it generates the bindings, and they are 100% synchronous. And their book didn't mention anything about size limit. |
@anurbol It sounds great. |
It was easier for me to work all these changes into a new PR - so I'm closing this in favour of #625 |
@anurbol is this what you had in mind for the follow-up PR? #106 (comment)