Parse and transform JS and TS files to allow them to be uploaded to Cloudflare #120
Conversation
RobbieMcKinstry
left a comment
RobbieMcKinstry
left a comment
There was a problem hiding this comment.
Speedy work! I have some change requests though
Cargo.toml
Outdated
| ] } | ||
| url = "2.5.4" | ||
| uuid = { version = "1.9", features = ["serde", "v4"] } | ||
| swc_common = { version = "14.0.2", features = ["tty-emitter", "sourcemap"] } |
There was a problem hiding this comment.
Probably don't need tty-emitter anymore!
src/adapters/cloudflare/mod.rs
Outdated
| if extension == "js" || extension == "ts" { | ||
| // Use processed/transformed bytes for JS/TS files | ||
| process_file(file_path)? |
There was a problem hiding this comment.
nuh uh, you should use the TypeBuilder like I did to construct a matcher that understands the filetypes. Burntsushi knows what he's doin', his code is a little more robust.
There was a problem hiding this comment.
That is to say, you'll have to make that change in manifest.files(), not at this location.
src/adapters/cloudflare/mod.rs
Outdated
| // Use original bytes for other file types | ||
| read(file_path).await.into_diagnostic()? |
There was a problem hiding this comment.
We probably don't have to update code that isn't ts or js.
There was a problem hiding this comment.
What about Python and Rust workers?
src/adapters/cloudflare/mod.rs
Outdated
| let file_bytes = if let Some(extension) = file_path.extension() { | ||
| if extension == "js" || extension == "ts" { | ||
| // Use processed/transformed bytes for JS/TS files | ||
| process_file(file_path)? |
There was a problem hiding this comment.
Does this result just get thrown away? It looks like it doesn't have any side effects. Same with read below. In the red diff, read returned a byte vec, but now its return type is unused.
src/adapters/cloudflare/mod.rs
Outdated
| let handler = Handler::with_tty_emitter( | ||
| swc_common::errors::ColorConfig::Auto, | ||
| true, | ||
| false, | ||
| Some(cm.clone()), | ||
| ); |
There was a problem hiding this comment.
I suspected we were going to drop the tty emitter. Is that just for errors or something? What's actually responsible for dumping the output file?
| // Ensure that we have enough parenthesis. | ||
| let program = module.apply(fixer(Some(&comments))); | ||
|
|
||
| to_code_default(cm.clone(), Some(&comments), &program) |
There was a problem hiding this comment.
I guess it's to_code_default?
src/artifacts/cloudflare/manifest.rs
Outdated
| let manifest_filenames = manifest_filenames(); | ||
|
|
||
| // Build the file tree walker. | ||
| // let walker = walk_builder(directory.clone()); |
There was a problem hiding this comment.
This is where the TypeBuilder should be used.
No description provided.