[IMP] owl-core: add types for Set and Map - #2016
Conversation
|
i think you should do something like this: ad592dd to avoid observing values when validating types |
Before this commit, validating inside a computation subscribes it to what it validates: an effect calling `assertType(state.items, t.array(t.number()))` re-runs on every write to the array, and a `t.customValidator` predicate reading a signal subscribes to that signal. This happens because the validators read the value through its proxy, which also allocates a per-key atom for every key they walk, even when no computation is active. This commit runs `validateType` untracked and hands each validation context the raw value: nothing a validator or a custom predicate reads is observed, and a reactive value is walked without allocating anything. Note that an issue now reports the raw value in `received`.
Before this commit, a set or a map could only be typed with `t.instanceOf(Set)`, which validates the container and nothing else: what it holds is unchecked, and the type derived from the schema is `Set<any>`. `t.customValidator` can check the content, but it keeps the type of the base it wraps, so the reader gets no completion. This commit adds `t.set(valueType?)` and `t.map(keyType?, valueType?)`, which validate every element, key and value, and resolve to `Set<V>` and `Map<K, V>`. A failing entry is reported by its iteration index (`0 > key`, `0 > value` for a map), as a map key can be any value. Note that the validation context gains `withEntry(key, value)`: a set element and a map key are not reachable by key on the validated value. Closes odoo#1939
c7e6bcc to
744438e
Compare
|
@ged-odoo but then the issue is generic and pre-existing, it should apply to all types, including custom validators that use signal internally, so I pushed a new commit to take this into account, but I'm not sure if we really need this. Is there anything actually observing while validate? If the validator is called in an effect/computed like in the tests, wouldn't it be the expected behavior that it actually re-validates on change? |
|
Should I push the PR without the no-observing commit, so we can already merge the Set/Map, and determine later/when we have time whether observing should be handled or not and how? |
Before this commit, a set or a map could only be typed with
t.instanceOf(Set), which validates the container and nothing else: what it holds is unchecked, and the type derived from the schema isSet<any>.t.customValidatorcan check the content, but it keeps the type of the base it wraps, so the reader gets no completion.This commit adds
t.set(valueType?)andt.map(keyType?, valueType?), which validate every element, key and value, and resolve toSet<V>andMap<K, V>. A failing entry is reported by its iteration index (0 > key,0 > valuefor a map), as a map key can be any value.Note that the validation context gains
withEntry(key, value): a set element and a map key are not reachable by key on the validated value.Closes #1939