Add type checking to items expression #69

CharString · 2023-11-28T09:39:29Z

Closes open-formulieren/open-forms#3596
Closes open-formulieren/open-forms#3597

sergei-maertens · 2023-11-29T13:55:43Z

src/context.ts

@@ -47,6 +48,43 @@ export interface DocumentTypeOption {
  };
 }

+export interface ServiceFetchConfiguration {


hrm, I'm not sure this belongs in the form-builder repository - does something break if you ignore serviceFetchConfiguration in the VariableDefinition type?

Perhaps it makes more sense to pass getInfernoLogicContext via the builder context rather than providing the inputs and creating this context? That way we can build that context in our backend JS and pass it to the builder, and the open-forms specific stuff is more decoupled from the formio domain for the builder? Ultimately you sort of initialize the builder with "here, this is the infernologic type checking context, use that for your type checks and please validate the things in the builder" without the builder itself needing to understand what infer needs (other than the function signature).

sergei-maertens · 2023-11-29T13:58:26Z

src/components/builder/values/items-expression.tsx


 const NAME = 'openForms.itemsExpression';

+const dataTypeForComponent = (component: AnyComponentSchema): JSONValue => {


would it make sense to add this to the infernologic library itself? In a contrib/formio package or something like that? That would also make it more easily shared between the builder & backend.

The backend depends on the builder, even if the feature flag is off.

This failed build illustrates why this code lives here: https://github.com/open-formulieren/formio-builder/actions/runs/7101208159/job/19328836339?pr=69

The exhaustiveness checks for components that have been added since the version the PR was based on. CI telling us we should explicitly specify what type these form components expect 💚

... to item expression Closes open-formulieren/open-forms#3596 Closes open-formulieren/open-forms#3597

.storybook/decorators.tsx

src/components/ComponentConfiguration.stories.tsx

sergei-maertens · 2023-12-05T13:59:03Z

src/components/ComponentConfiguration.stories.tsx

@@ -1081,7 +1095,7 @@ export const SelectBoxes: Story = {
        isSensitiveData: false,
        openForms: {
          dataSrc: 'variable',
-          itemsExpression: {var: 'someVar'},
+          itemsExpression: {var: 'current_year'}, // valid JSON, invalid expression


this makes the validation non-blocking, no? even if it's an invalid expression, it gets submitted and saved, that's what this assertion is doing. What's the reason for not preventing the <form> submission?

src/components/JSONEdit.tsx

src/utils/jsonlogic.ts

sergei-maertens · 2023-12-05T14:20:18Z

src/utils/jsonlogic.ts

+      }
+): JSONValue => {
+  // For now return example values as accepted by InferNoLogic
+  // But example values cannot distinguish arrays from tuples!


not even when using [...] as const?

Not sure what Chris and you meant here

I think the issue is that some expressions like ['foo, 'bar'] get inferred as string[] where-as they should be a two-tuple. If you define a fixed-length array of types, then it becomes a tuple in TS: https://www.typescriptlang.org/docs/handbook/basic-types.html#tuple

I was wondering if using the value as const construct would make TS infer it as a tuple rather than an Array

The problem is not TS, but open-formulieren/InferNoLogic/issues/5

At some point the returntype of this function will have to become some form of type annotation. Not just to distinguish array from tuple, but also to distinguish string from date, datetime etc.

sergei-maertens · 2023-12-05T14:21:42Z

src/utils/jsonlogic.ts

+    checkbox: true,
+    //@ts-ignore selectboxes always have component.defaultValue (this does work when rewritten as a lengthy switch/case)
+    selectboxes: component.defaultValue,
+    npFamilyMembers: {}, // TODO record type


this eventually becomes the same as selectboxes BUT it can also become a type: 'content' component if there's an error 😬

src/utils/jsonlogic.ts

Viicos · 2023-12-22T10:43:50Z

src/components/JSONEdit.tsx

@@ -46,6 +46,7 @@ const JSONEdit: React.FC<JSONEditProps & TextareaHTMLAttributes<HTMLTextAreaElem
    }

    updateValue(updatedData);
+    name && validateField(name);


@sergei-maertens we are hitting an issue here and don't know how to get things working:

name && validateField(name) was added because the updateValue call above did not trigger the validate callback here (even though validateOnChange is set to true).

The issue is data gets out of sync, as here updatedData corresponds to the latest version of the user input data, whereas adding a console.log in the parent validate callback shows an outdated value.

I've tried adding a call to the base Formik onChange:

const {onChange: formikOnChange} = getFieldProps(name); // Then calling `formikOnChange` in our custom `onChange`

But nothing works out

I still don't think that wiring up this validate behaviour directly in the component is the right approach. Especially because this JSONEdit component is also used to edit the entire form component, but it could also point at a different subtree that has nothing to do with JSONLogic.

IMO hooking up the validator belongs in the zod validation schema, where you can make use of .refine or .superRefine to invoke this validator:

const schema = z.object({ itemsExpression: z.record(z.unknown()).superRefine( ... // in the callback here you'd call the `infer` validator/checker ), });

This is cleaner indeed. Unfortunately I seem to be facing robertLichtnow/zod-formik-adapter#15, will have to look closer.

That seems weird since the (super)refine is used in the columns component validation. It could be that the error information is present, but just not displayed.

edit: hrm this might be why the validation errors are not shown onChange but only onBlur 🤔

I see it being called with some logs, but the error doesn't seem to be displayed.

…nLogic

…Logic

sergei-maertens · 2024-01-24T15:07:54Z

Sorry, postponing this :(

➕ Add InferNoLogic 0.1.1

447e53e

sergei-maertens reviewed Nov 29, 2023

View reviewed changes

✨[open-formulieren/open-forms#3597] Add type checking

0ca1be3

... to item expression Closes open-formulieren/open-forms#3596 Closes open-formulieren/open-forms#3597

CharString force-pushed the 3597-add-type-checking-to-items-expression branch 2 times, most recently from d33dd0f to 9613441 Compare December 5, 2023 14:12

sergei-maertens reviewed Dec 5, 2023

View reviewed changes

👌[open-formulieren/open-forms#3597] Process review

4b089d4

CharString force-pushed the 3597-add-type-checking-to-items-expression branch from 9613441 to 9c8e830 Compare December 6, 2023 17:22

CharString mentioned this pull request Dec 6, 2023

Add typecheck to "action sets the value" open-formulieren/open-forms#3667

Closed

CharString force-pushed the 3597-add-type-checking-to-items-expression branch from 9c8e830 to b7135f8 Compare December 6, 2023 17:38

Viicos reviewed Dec 22, 2023

View reviewed changes

[open-formulieren/open-forms#3597] Switch to a zod validation for jso…

b7e11e4

…nLogic

Viicos force-pushed the 3597-add-type-checking-to-items-expression branch from 09a25e8 to d672e94 Compare December 22, 2023 14:13

[open-formulieren/open-forms#3597] Handle exceptions added in InferNo…

414a9b0

…Logic

Viicos force-pushed the 3597-add-type-checking-to-items-expression branch 2 times, most recently from 8343b96 to 414a9b0 Compare December 27, 2023 15:11

Viicos added 2 commits December 27, 2023 16:30

[open-formulieren/open-forms#3597] Update to latest infernologic

9331697

[open-formulieren/open-forms#3597] Fix missing import

c1640f4

sergei-maertens closed this Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add type checking to items expression #69

Add type checking to items expression #69

CharString commented Nov 28, 2023

sergei-maertens Nov 29, 2023

sergei-maertens Nov 29, 2023

CharString Dec 5, 2023

sergei-maertens Dec 5, 2023

sergei-maertens Dec 5, 2023

Viicos Dec 22, 2023

sergei-maertens Dec 22, 2023

CharString Dec 22, 2023

CharString Dec 22, 2023

sergei-maertens Dec 5, 2023 •

edited

Loading

Viicos Dec 22, 2023

sergei-maertens Dec 22, 2023

Viicos Dec 22, 2023

sergei-maertens Dec 22, 2023 •

edited

Loading

Viicos Dec 22, 2023

sergei-maertens commented Jan 24, 2024


		const NAME = 'openForms.itemsExpression';

		const dataTypeForComponent = (component: AnyComponentSchema): JSONValue => {

Add type checking to items expression #69

Add type checking to items expression #69

Conversation

CharString commented Nov 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergei-maertens Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergei-maertens Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergei-maertens commented Jan 24, 2024

sergei-maertens Dec 5, 2023 •

edited

Loading

sergei-maertens Dec 22, 2023 •

edited

Loading