-
Notifications
You must be signed in to change notification settings - Fork 337
Use serde for metadata #285
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,8 @@ use std::path::{Path, PathBuf}; | |
use log::info; | ||
|
||
use crate::commands::build::wranglerjs::output::WranglerjsOutput; | ||
use crate::settings::binding::Binding; | ||
use crate::settings::metadata; | ||
use crate::terminal::message; | ||
|
||
// Directory where we should write the {Bundle}. It represents the built | ||
|
@@ -50,8 +52,9 @@ impl Bundle { | |
|
||
script_file.write_all(script.as_bytes())?; | ||
|
||
let metadata = create_metadata(self).expect("could not create metadata"); | ||
let mut metadata_file = File::create(self.metadata_path())?; | ||
metadata_file.write_all(create_metadata(self).as_bytes())?; | ||
metadata_file.write_all(metadata.as_bytes())?; | ||
|
||
// cleanup {Webpack} dist, if specified. | ||
if let Some(dist_to_clean) = &wranglerjs_output.dist_to_clean { | ||
|
@@ -109,31 +112,20 @@ pub fn create_prologue() -> String { | |
} | ||
|
||
// This metadata describe the bindings on the Worker. | ||
fn create_metadata(bundle: &Bundle) -> String { | ||
info!("create metadata; wasm={}", bundle.has_wasm()); | ||
fn create_metadata(bundle: &Bundle) -> Result<String, serde_json::error::Error> { | ||
let mut bindings = vec![]; | ||
|
||
if bundle.has_wasm() { | ||
format!( | ||
r#" | ||
{{ | ||
"body_part": "script", | ||
"bindings": [{{ | ||
"name": "{name}", | ||
"type": "wasm_module", | ||
"part": "{name}" | ||
}}] | ||
}} | ||
"#, | ||
name = bundle.get_wasm_binding(), | ||
) | ||
.to_string() | ||
} else { | ||
r#" | ||
{ | ||
"body_part": "script" | ||
} | ||
"# | ||
.to_string() | ||
bindings.push(Binding::new_wasm_module( | ||
bundle.get_wasm_binding(), | ||
bundle.get_wasm_binding(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. twice? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once is the name and the other the multipart part. |
||
)); | ||
} | ||
|
||
serde_json::to_string(&metadata::Metadata { | ||
body_part: "script".to_string(), | ||
bindings, | ||
}) | ||
} | ||
|
||
#[cfg(test)] | ||
|
@@ -167,14 +159,7 @@ mod tests { | |
let contents = | ||
fs::read_to_string(&bundle.metadata_path()).expect("could not read metadata"); | ||
|
||
assert_eq!( | ||
contents, | ||
r#" | ||
{ | ||
"body_part": "script" | ||
} | ||
"# | ||
); | ||
assert_eq!(contents, r#"{"body_part":"script","bindings":[]}"#); | ||
|
||
cleanup(out); | ||
} | ||
|
@@ -233,16 +218,7 @@ mod tests { | |
|
||
assert_eq!( | ||
contents, | ||
r#" | ||
{ | ||
"body_part": "script", | ||
"bindings": [{ | ||
"name": "wasmprogram", | ||
"type": "wasm_module", | ||
"part": "wasmprogram" | ||
}] | ||
} | ||
"# | ||
r#"{"body_part":"script","bindings":[{"type":"wasm_module","name":"wasmprogram","part":"wasmprogram"}]}"# | ||
); | ||
|
||
cleanup(out); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
use serde::Serialize; | ||
|
||
#[derive(Serialize, Debug)] | ||
#[serde(tag = "type")] | ||
pub enum Binding { | ||
#[allow(non_camel_case_types)] | ||
wasm_module { name: String, part: String }, | ||
} | ||
|
||
impl Binding { | ||
pub fn new_wasm_module(name: String, part: String) -> Binding { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems odd; why would we want a wasm-specific method on the binding trait? or am i reading this wrong? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea is too add for each binding type a method (like a factory, constructor), see https://github.com/cloudflare/wrangler/pull/215/files#diff-deeb786858380f08f8f4d40680620a9a if it makes more sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok that makes sense, thank you. |
||
Binding::wasm_module { name, part } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
use serde::Serialize; | ||
|
||
use crate::settings::binding::Binding; | ||
|
||
#[derive(Serialize, Debug)] | ||
pub struct Metadata { | ||
pub body_part: String, | ||
pub bindings: Vec<Binding>, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,4 @@ | ||
pub mod binding; | ||
pub mod global_user; | ||
pub mod metadata; | ||
pub mod project; |
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 know this didn't originate from this PR, but why do we write the metadata to a file?
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 think its useful if you want to use wrangler as a "build-only" tool
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.
But metadata isn't used unless you're publishing to Cloudflare, so we support building with wrangler and then publishing with curl or something?
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.
Maybe as part of a CI process where the CI handles upload because wrangler doesn't support complex config for handling staging environments etc. I think it's useful for all "files" that we upload for multipart to end up on disk somewhere for this reason.
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.
does it stay on disk? or is it cleaned up later?
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.
There was some back and forth with the metadata file lately, see #276. The file is store in ./worker as part of the worker "bundle", it is persistent.
I'm sure some people will commit their worker, it make sense to have a consistent state including the metadata, it's useful for the preview and debugging too.