"Fix" capnproto serialisation of undefined
values, closes #371
#377
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously passing
compatibilityDate: undefined
orcompatibilityFlags: undefined
would throw.This is because the
undefined
s would make it to theencodeCapnpStruct
function:miniflare/packages/tre/src/plugins/core/index.ts
Lines 172 to 173 in b0e1d06
encodeCapnpStruct
distinguishes between{ compatibilityDate: undefined }
and{}
. Callingtre/src/runtime/config/sserve-conf.capnp.js/Worker#setCompatibilityDate(value)
with anundefined
value throws.The problem is that
undefined
may be needed in some cases to signal that a function should be called, e.g.sserve-conf.capnp.js/Worker_Binding_Type#setR2Bucket()
.This "fix" checks if the function actually accepts a
value
before calling it. Because we don't have runtime type information, this is done by looking at the function's source code dynamically. 🤮Writing out this description has given me an idea for an alternative. We could replace the
undefined
s insserve-conf.ts/Worker_Binding_Type
with a specialSymbol
to signal that we wanted to call the function with no arguments. Then just ignore allundefined
values inencodeCapnpStruct
? What do you think?