-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
WIP Renamed node-bindings and added a dedicated entry point for wasm #9768
Conversation
6b9a47c
to
0d32a6e
Compare
c33bae7
to
b0b01b5
Compare
build.rs for napi build.rs for napi build.rs for napi dylib Reverted changes to napi package Update napi wasm build
0d32a6e
to
25a5ad4
Compare
b0b01b5
to
6eaa135
Compare
Why? Isn't parcel_wasm mostly a duplicate of the napi bindings? There's only a few differences that were covered by conditional compilation before. |
} | ||
|
||
// We don't currently call functions from multiple threads, but we'll need to change this when we do. | ||
unsafe impl Send for FunctionRef {} |
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.
Why do you need this if you don't call it from multiple threads?
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.
Good point, I'm just moving existing code and not making changes to the sources
crates/parcel_wasm/src/resolver.rs
Outdated
#[napi(constructor)] | ||
pub fn new(project_root: String, options: JsResolverOptions, env: Env) -> Result<Self> { | ||
let fs = { | ||
let fsjs = options.fs.unwrap(); |
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.
you can just fail here ; options.fs.unwrap_or(Err...)?
|
||
use crate::function_ref::FunctionRef; | ||
|
||
pub struct FileSystemWasm { |
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.
was a safe implementation of this
crates/parcel_wasm/src/resolver.rs
Outdated
let mut res = self.resolver.resolve_with_options( | ||
&options.filename, | ||
Path::new(&options.parent), | ||
match options.specifier_type.as_ref() { |
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.
nit, assign these to variables so that we have names
crates/parcel_wasm/src/resolver.rs
Outdated
)) | ||
} | ||
}, | ||
if let Some(conditions) = options.package_conditions { |
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.
same here
crates/parcel_wasm/src/resolver.rs
Outdated
mode: u8, | ||
resolver: parcel_resolver::Resolver<'static, FileSystemWasm>, | ||
} | ||
|
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.
you could break up a lot of these functions
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.
I don't understand why we're duplicating all this code
I'm thinking of abstracting those peices out on their own and share them between For instance, the Resolver implementation can be moved into This would avoid duplication, allowing the Resolver code to remain largely unchanged whist splitting the wasm and nodejs entrypoints up. In the end state the binding layer will be significantly lighter so this will be replaced |
But why do we even need separate entry points for them in the first place if they are both using napi? |
↪️ Pull Request
node-bindings
toparcel_napi
src/parcel_v2
parcel_v2
or migrate code into new context as we progressparcel_wasm
package to be an entry point forwasm
consumerswasm
code fromnode-bindings
intoparcel_wasm
💻 Examples
🚨 Test instructions
✔️ PR Todo