Refactor to use functional paradigm

[kettanaito]

This pull request introduces breaking changes, since it essentially re-writes the form-fields communication interface and validation logic - two pillars of the form solution.

• Relates to Formless fields #216
• Fixes Introduce custom Cypress commands to ease integration testing #256

Overview

What

This pull request introduces changes that refine the existing architecture to use a more functional approach. By the latter I mean a preference for small pure functions, which can be easily swapped or reused.

Why

1. Simplifies the architecture.
2. Makes the architecture more extendable, allowing such features as Formelss fields
3. Makes individual pieces of functionality easier to test and type annotate
4. Potentially increases the performance of the library

Roadmap

Architecture

• Add a few functions defining relation between given functions (parallel, seq)
• Add a collection of pure functions responsible for field record operations
• Use Immutable.Record to handle field record instead of manual composition in createField
• Consider high function composites from the recordUtils to complexly handle field interactions (i.e. handleFieldChange), to be used by the formless fields
• Refine existing field handlers on Form level (simplify)
  • Isolate Form.validateField into a pure function

Validation

• Add a unified interface for creating a validation result
• Add a unified interface for creating a validator (resolver) arguments
• Add a unified interface to get the rules relative to the field from the given schema
• Add a unified interface to create a rejected rule entity
• Add pure independent interface for field validation
• Get rid of ValidationType class
• Add a function to determine the necessity of the validation of a certain type (replacement for ValidationType.shouldValidate()

Validation messages

• Refine validation messages logic to use pure functions
• Propagate "extra" parameters from async response to the message resolver
• Integrate getMessages into validateAndReflect

Misc

• Reactive props: Adjust getRxProps according to the updated field Record

29.04

• Propagate rejected rules for rejected applyFormRules to the top-level validation result
• Consider making the logic around shouldValidate smarted:
  • Make types argument optional for validateByType function. When it's not provided, call getNecessaryValidationTypes on the given fieldProps, and build a validators sequence based on the received validation types array.
  • Adjust Form logic not to pass any types when using Form.validateField() by default, so that the changes above get place (automatic determining of the necessary validation types)

05.05

• Add unit tests for existing utility functions.
  • camelize
  • createSeq
  • debounce
  • dispatch
  • enforceArray
  • getComponentName
  • flushFieldRefs
  • flattenDeep
  • isset
  • recordUtils
  • makeCancellable
• recordUtils.createField figure out how initialValue is set. Whether on usage level, or class level.
• (Integration) Fix reactive props tests

07.05

• Provide better validation types propagation to reflectValidationResult. I do not like how validate returns both validation result and types being applied.

08.05

• Reactive props require to re-validate the field with the force parameter. Need to add force logic support to validation algorithm.

12.05

• Bundle Ramda properly
• Added ruleKeyPath to a rule and rejected rule to make getting error messages easier
• Add explicit validation types as a parameter to validate function
• Add explicit force to force any provided validators
• Create composite validate and reflectValidation method
• Check what is returned from the validator function when no validation is needed
• Create checkRequiredEmpty validator and integrate it into validateSync chain
• Consider !value && !required as a part of shouldValidateSync()
• applyFormRules handle call when no rules are present
• applyFormRules compose funcs based on which rules are present

19.05

• Ensure sync validation is bypassed on fieldBlur if field is already invalid
• Ensure no necessity of validation is handled properly. It doesn't validate, doesn't set the expected to true either.

22.05

• Separate result reducing logic into two separate functions: reduceResults and reduceResultsWhile
• Refine if shouldValidateX is necessary when we have dynamic rules list composition via listOf(addWhen)

27.05

• Fix Form.submit() to be prevented if there are any unexpected fields
• Fix "Default debounceTime" integration test case. That one is broken because validation behavior was changed and now the field's validity props are reset on each change. I think that affects when debounce is fired.

8.06

• Investigate why blurring from the invalid field resets its validity state

16.06

• Sync validation, "Form.props.rules" scenario. Reactive field (the second one) works incorrect. (fixed by 8e172bf) Steps to reproduce:

1. Type "some" into first field.
2. Type "some" into the second field. See it becomes valid.
3. Type "somes" into the second field. It's invalid now.
4. Type "somes" into the first field. The second becomes valid and then suddenly invalid.

23.06

• Uncomment and ensure proper integration test scenario for Reactive props "Field.props.rule behaves as a reactive prop" (4ddb399)

Asynchronous validation

• Bring async nature to validation (3031b82)
• Include validateAsync into default validator chain
• Ensure async validation is called even if sync validation was unnecessary
• Ensure validation messages for async rejected rules are resolved correctly (00c6cdf)
• Fix pending validation cancellation
• Async validation integration tests are passing
• validateAsync being the last entry in the validation chain, if proves unnecessary (or falsely returns true when it shouldn't) it makes the field expected, when it shouldn't be (take interdependent rxprops scenario) (3ed6df5, 79adaed)
• Fix mixed validation integration tests (somewhat unclear. was fixed, but runs with different results. requires more testing)
• Fix AJAX pre-filling async validation scenario

Misc

• Propagate "validating" state during the field blur once the validation occurred (have intermediate fields update) Declined for now. Should be introduced as a separate new feature.

Functional

• Re-write recordUtils to be curried functions
• Re-write cumbersome recordUtils calls to use functional composition:

const someTransfomer = compose(
  recordUtils.setFocus(false),
  recordUtils.updateValue('foo')
)

• Fix console errors on "Reactive props" integration test scenarios. Cannot read property "getIn" of undefined.

Final

• Refine the necessity of old unit test cases
• Clean up

[kettanaito]

Update

I am currently working on validation types. Trying to unify them, make their declaration more clear.

Changelog

• Created a dedicated validationTypes file to contain the collection of the available validation types.
• Each validation type has the following properties:
  • shouldValidate(fieldRecord, schema) function determines whether the current validation type is needed for the given field/schema.
  • validator(resolverArgs) executes the validation itself, returns TValidationResult.

I am experiencing a weird issue during mapToSingleResult, where, despite which validation types are relevant to the given field, it reduces them all. So I get some pending Promise from validateAsync, I believe. That breaks the returned Object and, therefore, expected is undefined.

Update: That pending promise was cause because I made the reducer function in mapToSingleResult to be async, in order to await the async validation. When async is removed, it started to validate sync properly.

[kettanaito]

Currently still experiencing that unnecessary validation issue. May be related to wrong handling of expected: true returned by the validate function when no validation is needed (which is correct, no validation needed - the field is still expected, but its validity must not change).

Update: Solved that by explicitly returning undefined from validate() when no validation is needed. Returning expected: true is not quite proper, since then form needs to distinguish whether to reflect that validation result in the validity state. Returning undefined, on the other hand, allows to do something as simple as this:

if (!validationResult) {
  return;
}

[kettanaito]

Update

Investigating on the combined validation scenario, and the issue that async validation is triggering after sync invalid field, which results into distorted validation result.

To understand the issue, it's important to understand the chain of actions affecting the validation:

1. Field value change. Upon the change, sync validation is triggering and resulting into proper validation result.
2. Field blur. This is the part that breaks the validation combination. The thing is, when the field has been already validated sync upon value change, validation call that triggers onBlur computes shouldValidateSync to false, that returns explicit null from validateSync. Explicit null is not treated as falsy value during the validation chain, so it bubbles to async validation.
3. Async validation. There is nothing really wrong with it, so far.

The reason for combined validation not working is unnecessary sync validation triggered by onBlur.

Solution

My previous assumption of unnecessary validSync check during shouldValidateAsync was wrong. The fix was to prevent async validation in case the field was validated sync and is not valid:

const validSync = ({ fieldProps }) => {
  return fieldProps.validatedSync ? fieldProps.validSync : true
}

This way async validation relies on the reflected validation result. Even when sync returns explicit null during onBlur validation, the function above resolves properly.

[kettanaito]

Okay, I am experiencing a weird issue with Cypress typing into inputs again. Sometimes it types everything except the first letter. That, obviously, fails the test, which expects the value match. At the moment this randomly fails "Combined validations" scenario.

When that scenario is run alone, it passes always. That makes me thinking that it may be an issue with accumulation of scenario calls. Maybe some load or something. Possible on both form and Cypress levels. I usually play with timeouts to eliminate overload issues, so that may be a good place to start.

[kettanaito]

Update on AJAX pre-filling scenario

I have analyzed the issue and conclude that the reason was due to concurrent state updates. There are two fields which get the same controlled value upon some async operation. Each value change fires:

fieldChange (e) -> listener -> form.handleFieldChange -> onUpdateValue (uFW) -> validateField/debouncedValidate -> uFW

Concurrency comes in onUpdateValue and its updateFieldWith. Each field bubbles with form state update when the value of other fields in the this.state.fields at the update point of time is empty. This results than once the update for second field comes in, it bears an empty value of the first field, and reset its value upon fields update. Of course, validating an empty field renders it invalid, which was the unexpected behavior of that test case.

That was caused due to the following dispatch of appropriateValidate:

appropriateValidate({
  chain: [validateSync],
  fieldProps: updatedFieldProps,
  form
})

The issue with this, is that in case of debounced validation appropriateValidate references to fieldProps.debouncedValidate, which calls form.validateField under the hood. The latter method has an implicit logic built in where unless you specify forceProps: true, it would try to grab the actual field record from the fields state at the call time by fieldProps.fieldPath. After that it would pass a field record with an empty (reset due to concurrency uFW) value to validation function.

At the moment, I haven't touched this concurrency issue, but have changed the logic of shouldDebounce. Only debounced validation referenced to form.validateField(), which bears the concurrency. Now pre-filling will fall into logic of plain validateField function, without debouncing, thus eliminating this issue.

I do understand that concurrent state updates must be dealt with, but haven't come up with a stable solution for that.

[kettanaito]

I will try to fix unstable typing issue in Cypress by doing the following:

1. Set global typing delay to some reasonable amount (~30ms)
2. Set cy.wait() before the problematic tests to ensure everything is bootstrapped and works properly.

[kettanaito]

Update

validateField from field change (debounced) was dispatched without fields and caused an exception during the validation execution. fields reference is omitted explicitly during debounced validate calls due to debounced nature of the validation. Supplying fields there would mean that their reference may be out of date. Therefore, the following change has been implemented on the createRuleResolverArgs level:

const fields = args.fields || args.form.state.fields

I will see how that affects the tests, since it, essentially, is the same as specifying the fields directly in the appropriateValidate() call.