Add additional fields to OneOfBlock type #2878
Conversation
| name, | ||
| blockMeta: new Meta(OneOfBlockData), | ||
| migrate, | ||
| }) as unknown as OneOfBlock<BlockMap, BlockData, BlockInput>; |
There was a problem hiding this comment.
I've added the as cast to suppress the following error:
Type 'Block<OneOfBlockDataInterface<BlockMap, BlockDataInterface>, OneOfBlockInputInterface<BlockMap, SimpleBlockInputInterface>>' is not assignable to type 'OneOfBlock<BlockMap, BlockData, BlockInput>'.
Types of property 'blockDataFactory' are incompatible.
Type 'BlockDataFactory<OneOfBlockDataInterface<BlockMap, BlockDataInterface>>' is not assignable to type 'BlockDataFactory<OneOfBlockDataInterface<BlockMap, BlockData>>'.
Type 'OneOfBlockDataInterface<BlockMap, BlockDataInterface>' is not assignable to type 'OneOfBlockDataInterface<BlockMap, BlockData>'.
Type 'OneOfBlockDataInterface<BlockMap, BlockDataInterface>' is not assignable to type 'BlockData'.
'OneOfBlockDataInterface<BlockMap, BlockDataInterface>' is assignable to the constraint of type 'BlockData', but 'BlockData' could be instantiated with a different subtype of constraint 'BlockDataInterface'.ts(2322)
I don't think it's possible to fix this without adding such a cast somewhere, so I decided to invest little effort and don't type options, internal classes etc. correctly. Do you think this okay?
There was a problem hiding this comment.
I'm not aware enough of how hard it is to have proper typing here, due to my lack of knowledge of the code-base, but I would say this should really be avoided 😅
There was a problem hiding this comment.
What do you mean? Avoid the as cast?
There was a problem hiding this comment.
I'm not sure if this can be achieved when using generics though
There was a problem hiding this comment.
I just checked out the code for it and I'm not familiar with the code here either but I agree with Obi. Even if I don't like the cast, I haven't come up with anything better here, if that's even possible. I could only imagine that Niko will come up with something else here.
There was a problem hiding this comment.
We added support for additional fields in the OneOfBlock in #1666. However, the additional fields weren't added to the type.
TODO
Additional information
First commit adds a test and demonstrates the bug, second commit solves the bug.