Add typechecking to itemsExpression #3610
Conversation
CharString
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3610 +/- ##
=======================================
Coverage 95.97% 95.97%
=======================================
Files 680 680
Lines 21890 21891 +1
Branches 2532 2532
=======================================
+ Hits 21008 21009 +1
Misses 611 611
Partials 271 271 ☔ View full report in Codecov by Sentry. |
CharString
force-pushed
the
3596-add-typechecking-to-keuzeopties-expressie
branch
from
300398a to
001c6a5
Compare
Forces expressions of type `string[][]`. Accepting `[value: any, label: string][]` would require open-formulieren/InferNoLogic#5 To allow ```ts type Item = [value: any, label: string] | {value: any, label: string} type ValidItemExpression = Item[] ``` open-formulieren/InferNoLogic#6 and even a union type!
CharString
force-pushed
the
3596-add-typechecking-to-keuzeopties-expressie
branch
from
001c6a5 to
3986a89
Compare
| const result = infer({'===': [logic, expected]}, {}); | ||
| return result.startsWith('result type:') || result; |
There was a problem hiding this comment.
infer API must change. This is ugly. Want to have more info from other tickets before making a decision and releasing a new npm package.
| await expect(page.locator("css=.formio-dialog-content")).to_be_visible() | ||
| await expect( | ||
| page.locator('css=[data-component-key="openForms.itemsExpression"]') | ||
| ).to_contain_text("Error") |
There was a problem hiding this comment.
Contents of "Error" will change. These assertions just assert that type check has occurred and accepts correct expressions.
Testing for the better error messages belongs in a unittest, E2E is way to slow and expensive for that.
SilviaAmAm
requested review from
sergei-maertens and
SilviaAmAm
and removed request for
sergei-maertens
| await editor.type( | ||
| '{"map": [{"var": "somevar"}, [{"var": ""}, {"cat": ["Optie ", {"var": ""}]}]]}' | ||
| ) |
There was a problem hiding this comment.
How does it know that {"var": ""} in the context of {"var": "somevar"} (is this checked that it is of type array?) is a string?
There was a problem hiding this comment.
I don't know the exact order, but it follows from
{"cat": ["Optie ", {"var": ""}]}- the list of options is of
string[][]/list[list[str]], therefore the result of the"map"is also of that type.
It's why the validateLogic passed the expression into
const result = infer({'===': [logic, expected]}, {});with expected = [["", ""]] and logic = (in this case) the "map"
It uses the fact that "===" is a Callable[[T, T], boolean], so the parse of the result from the infer call is a lot easier. infer({"===": [{"var": "a"}, {"var": "b"}]}) does not imply that a and b have the same value, just the same type.
infer shouldn't return a string, of course, but before changing it, I like to have more use cases.
|
Converted it back to draft so we don't get distracted by this PR - if I understood it correctly from the daily, the work is now happening in https://github.com/open-formulieren/formio-builder |
sergei-maertens
deleted the
3596-add-typechecking-to-keuzeopties-expressie
branch
Closes #3596