Regenerate email bindings with per-mono generics - #1058
Conversation
Merging this PR will not alter performance
|
|
Oh I thought ts-gen was used for way more than just email and flagship, I should probably just fold this into the first PR |
7167988 to
1e5edb4
Compare
76e555a to
4437aac
Compare
1e5edb4 to
795696c
Compare
4437aac to
b5eefaa
Compare
795696c to
0a5827b
Compare
b5eefaa to
8be4ab2
Compare
0a5827b to
d3ac101
Compare
guybedford
left a comment
There was a problem hiding this comment.
Verified locally: chomp build:types is a clean no-op on this commit (deterministic, and the ts-gen bump leaves flagship.rs unchanged), send_email.spec.ts + flagship.spec.ts pass, and the glue collapses (&str, &str, String) into a single new EmailMessage(...) import as expected.
Requesting changes for two things:
-
worker-sys/src/types/email.rsis a dead file — it is never declared (worker-sys/src/types.rshas nomod email;), it was left behind by #996. Rather than adding a second ts-gen invocation to keep regenerating it, delete the file and drop the second command. -
This is the first per-mono regeneration of an already-released binding, and it breaks
&Stringcallers.JsStringLikeis sealed toString,&str,JsString,&JsString; with a concrete&strparameter&Stringderef-coerced, through a generic bound it does not:
error[E0277]: the trait bound `&std::string::String: JsStringLike` is not satisfied
--> EmailMessage::new(s, s, s) // s: &String
The example diffs in this PR (new(&to, &from, &raw) -> new(to, from, raw)) are exactly that break, and worker::EmailMessage::new, SendEmailBuilderBuilder::text(&body) etc. are public API. This needs a wasm-bindgen PR adding IntoWasmAbi / OptionIntoWasmAbi / JsStringLike for &String (delegating to &str) before we regenerate existing bindings with per-mono generics, otherwise every binding we convert will hit the same regression. This PR should then land on top of that submodule bump with the example changes reverted.
| --export cloudflare:email \ | ||
| --external "Env=crate::Env" \ | ||
| --external "ExecutionContext=crate::Context" | ||
| ts-gen --input types/email.d.ts --output worker-sys/src/types/email.rs \ |
There was a problem hiding this comment.
worker-sys/src/types/email.rs isn't wired into worker-sys at all (no mod email; in worker-sys/src/types.rs) — it's an orphan from #996. Delete the file instead of adding this second invocation.
| .map_err(|e| Error::RustError(e.to_string()))?; | ||
|
|
||
| let reply = email::EmailMessage::new(&to, &from, &raw)?; | ||
| let reply = email::EmailMessage::new(to, from, raw)?; |
There was a problem hiding this comment.
This change is required because &String no longer satisfies the generic bound — which is a source break for downstream users of the released EmailMessage::new. See the review summary: this needs JsStringLike for &String upstream in wasm-bindgen first, after which this diff (and the send-email / test ones) can be reverted.
| pub fn set_filename(this: &EmailAttachment, val: &str); | ||
| pub fn set_filename<S: ::wasm_bindgen::JsStringLike>(this: &EmailAttachment, val: S); | ||
| #[wasm_bindgen(method, getter, js_name = "type")] | ||
| pub fn type_(this: &EmailAttachment) -> String; |
There was a problem hiding this comment.
Nit (ts-gen): type__js_string has a double underscore from type_ + _js_string. The keyword-escape underscore should be stripped before suffixing so this becomes type_js_string.
Summary
JsStringLikeparameters andJsStringgetter variantsts-gento include the typed-array dictionary-helper ABI fixStack
Validation
cargo test -p worker --target wasm32-unknown-unknowncargo check -p worker-sys -p worker --target wasm32-unknown-unknown