Skip to content

Conversation

@eshiota
Copy link
Collaborator

@eshiota eshiota commented Oct 17, 2025

Currently all custom JSON Logic operators are applied ad-hoc when the validation functions are executed, and then removed afterwards. While this makes sense from the point of view of simulating a deterministic call to JSON Logic, other places on the form's lifecycle require these custom operators to be available.

This means that if a form is loaded with an initial value where a property needs a computed value that relies on a custom operator, the operator won't be available (as it's only added when a validation happens), thus throwing an error.

This PR simplifies the logic by adding all custom operators at once when the createHeadlessForm is called, which happens only once per page load.

@eshiota eshiota self-assigned this Oct 17, 2025
Copy link
Collaborator

@danielcardoso5 danielcardoso5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eshiota needs tests fixing

@eshiota
Copy link
Collaborator Author

eshiota commented Oct 20, 2025

I'm fixing the test and I kinda get why this was previously implementing trying to make the createHeadlessForm more deterministic: if this is not done, then all calls to it (either on a page with multiple forms, or over multiple calls across multiple pages) will end up sharing and accumulating operators.

For most cases this should be fine, but it could cause nasty runtime bugs where subsequent calls impact what was previously defined, or there could be cases where a form works only because of another call introduced the necessary operator.

I'll go back to the drawing board and keep the setup-and-teardown strategy in a way that works on all necessary parts.

@eshiota
Copy link
Collaborator Author

eshiota commented Oct 20, 2025

@danielcardoso5 I've opened a new PR: #238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants