o1js
o1js copied to clipboard
o1-labs
Turn on `noUncheckedIndexAccess` TS config
This would be very nice for type soundness: https://www.typescriptlang.org/tsconfig#noUncheckedIndexedAccess
For someone who's motivated 💀
I don't think it would be worthwhile to refactor all of this code to check for index existence. I think a better approach will be to apply non-null assertions, and––from here on out––take advantage of noUncheckedIndexAccess. @mitschabaude want to assign this to me?
@harrysolovay if i understand you correctly, I agree: in most cases the initial fix should be something like assert(item !== undefined) before using the item that was obtained with an index access -- instead of refactoring the array to a tuple type / checking that the key is in the object.
I'm happy to see you having a go at this. Since it's a big refactor, it could also be done iteratively in several PRs: make more code handle the unchecked index access without yet committing the TS config change.
By "non-null" assertion I mean the operator.
declare const value: string | undefined
value! // `string`
We can apply these (!) pretty much everywhere there's a TS error stemming from potentially-undefined value access. I believe a large portion of these will need to be applied to the generated bindings of o1js-bindings. Might make sense to tweak the tsconfig within that directory alone.
By "non-null" assertion I mean the operator.
oh I see. I don't particularly like that operator, because it's neither type-safe nor a runtime assertion. It's like as <the type without null or undefined>. Using that operator everywhere is equivalent to not toggling noUncheckedIndexAccess on, which is fine as an upgrade path but not where I want to be headed.
I believe a large portion of these will need to be applied to the generated bindings of o1js-bindings.
No, we don't generate any TS code apart from type definitions. Bindings artifacts are JS
But yeah as a quick first step to enable noUncheckedIndexAccess, applying the ! operator makes a lot of sense