wrap-cli icon indicating copy to clipboard operation
wrap-cli copied to clipboard

replace Option with Box | null in wasm

Open fetsorn opened this issue 3 years ago • 3 comments

Optional base types are currently implemented as an Option container in wasm-as and schema/bind. But to omit optional fields from the function parameters we need a | null union type. So the user has to wrap Int in a reference type to be able to omit it, like Number | null.

# optional fields are `Number | null` instead of `Option<i32>`
type Number {
    value: Int!
}
type Module {
   # opt requires Option.None<i32> if omitted
   foo(opt: Int): Boolean
   # opt can be omitted
   bar(opt: Number): Boolean
}

Option was introduced in #944, but now it seems more important to omit function parameters than to provide nice Optional API, like "isSome()". We should replace Option<i32> with Box<i32> | null.

fetsorn avatar Aug 10 '22 16:08 fetsorn

@fetsorn why not do Option<T> | null? Also I believe that in AssemblyScript if a property is | null you still have to list that property, and cannot omit it entirely. Would do some checking to make sure this is the case.

dOrgJelli avatar Aug 10 '22 19:08 dOrgJelli

If we go for omittable fields, Box<T> is more straightforward.

Box is used to wrap primitive type such u32 which cannot be null. After boxing, the primitive type can be used like a reference type, and such as T | null syntax can be used. -- as-container docs

To access the API behind 'Option<T> | null' we'd have to first unwrap '| null', and then only a small portion of the API would be useful without Option.None, which could be misleading.

fetsorn avatar Aug 12 '22 10:08 fetsorn

Okay I can definitely see the point here, I’m in favor of using ‘Box<T> | null’ instead of Option

dOrgJelli avatar Aug 14 '22 07:08 dOrgJelli