Rust rewrite #52
Rust rewrite #52
Conversation
…hey can fix it for me
|
Should we use the same rustfmt settings as in the monorepo? |
|
@devongovett Sure, feel free to update it |
| return ctx.env.create_string(source_content); | ||
| } | ||
| None => { | ||
| return ctx.env.create_string(""); |
There was a problem hiding this comment.
Should this throw an error or return null maybe?
There was a problem hiding this comment.
This is intentional to mimic the c++ logic. Might make some breaking changes in a follow-up
| return ctx.env.create_string(name); | ||
| } | ||
| Err(_err) => { | ||
| return ctx.env.create_string(""); |
There was a problem hiding this comment.
Same with this one. I guess this is ignoring the error atm
There was a problem hiding this comment.
Same consistency w/ cpp implementation
parcel_sourcemap_node/src/lib.rs
Outdated
| let this: JsObject = ctx.this_unchecked(); | ||
| let source_map_instance: &mut SourceMap = ctx.env.unwrap(&this)?; | ||
|
|
||
| // TODO: Figure out a more optimal way of handling typed arrays... |
There was a problem hiding this comment.
I believe there is JsTypedArray/JsArrayBuffer: https://github.com/napi-rs/napi-rs/blob/e4753b7c1d839ab53de42107aeb5395db8749431/napi/src/js_values/arraybuffer.rs.
There's also JsBuffer if we wanna use Node buffers. https://napi.rs/concepts/js-values#jsbuffer
There was a problem hiding this comment.
Yeah been discussing how to properly tackle this with the napi-rs maintainer, gonna update that... it's really hitting performance atm
src/SourceMap.js
Outdated
| @@ -13,15 +14,18 @@ export default class SourceMap { | |||
| /** | |||
| * @private | |||
| */ | |||
| projectRoot: string; | |||
| projectRoot: any; | |||
There was a problem hiding this comment.
probably from some experimenting and flow complaining, reverted that...
| source: './helloworld.coffee', | ||
| generated: { line: 2, column: 2 }, | ||
| original: { line: 1, column: 0 }, | ||
| source: 'helloworld.coffee', |
There was a problem hiding this comment.
This change was intentional?
There was a problem hiding this comment.
Yes, wanted to make it follow the same logic as source-map library, esbuild, ...
There was a problem hiding this comment.
I can revert it very easily
|
@devongovett Thanks, will update readme and resolve your comments |
This PR is a ground-up rewrite of this library in Rust instead of C++.
This should result in a more stable and easier to read codebase, with the advantage of having access to the vast amount of libraries that are already available for Rust.
Fixes #54
TODO: