Skip to content

Introduce patched state update #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 21 commits into from
Closed

Conversation

kettanaito
Copy link
Owner

@kettanaito kettanaito commented Dec 6, 2018

Description:

Adjust Form state updates to be applied in patches. Event subscription listens to patched update events dispatched, accumulates them, and updates the Form state with the respective chunk information. This eliminates obsolete state properties, which are properties irrelevant to the occurring update (i.e. value property being updated during blur).

In comparison, previously, a value change event produced the entire next field state:

{
  value: 'nextValue',
  name: 'fieldOne',
  valid: true,
  ...
}

With patched state updates the value change event will produce only the following patch:

{
  value: 'nextValue',
  pristine: false,
}

The state patch is then deep merged via a single event handler into the existing Form state.

Tickets:

Roadmap:

  • Introduce synchronous way of applying state patches
  • Introduce asynchronous way of applying state patches
  • Deprecate Form.updateFieldsWith
  • Deprecate Form.fieldsDidUpdate
  • All tests are passing
  • Obsolete value of value during field change (fixed by 223ed93)
  • Simplified validation result reflection
  • Fixed a bug that a field rule rejected with no error displayed, when the error was set in the messages
  • Add integration scenario for error message for Field.props.rule error message (name/type/general)
  • (Bug) "RangeError: Maximum call stack size exceeded" upon form.reset()
  • Solve intermediate state composition issue (and recordUtils call signatures)

@kettanaito kettanaito changed the title WIP Fix reactive prop values of concurrently updated fields WIP Introduce chunked state update Dec 11, 2018
@kettanaito kettanaito force-pushed the 352-reactive-concurrency branch from a8cd0df to 7a114e4 Compare December 12, 2018 14:41
@kettanaito kettanaito changed the title WIP Introduce chunked state update WIP Introduce patched state update Dec 12, 2018
@kettanaito
Copy link
Owner Author

These changes may not fully solve the concurrency issue, but they prepare a great background for such concurrency handling, and open new possibilities of building detailed dev tools.

@kettanaito kettanaito force-pushed the 352-reactive-concurrency branch from 92323f2 to e81c188 Compare December 27, 2018 11:26
@kettanaito kettanaito force-pushed the 352-reactive-concurrency branch 2 times, most recently from 76fe898 to 7ca96e5 Compare January 22, 2019 14:30
@kettanaito kettanaito added scope:architecture Changes affect architecture. refactoring enhancement Enhances existing functionality. labels Jan 22, 2019
@kettanaito kettanaito force-pushed the 352-reactive-concurrency branch from 7ca96e5 to 42a3bd4 Compare January 23, 2019 12:02
@kettanaito
Copy link
Owner Author

kettanaito commented Jan 29, 2019

There is currently an issue with the value change, as it sometimes returns obsolete (accumulated) value after quick clean/type. This has been common before, and was solved, unfortunately this issue is back now.

Most likely there is an issue during field change/blur related to field state that is passed to the validation. Obsolete value most probably comes from the postponed validation.

Update: The issue was caused by the Form.validateField method returning the entire next state of a field. That conflicted with the properties that didn't need an update (i.e. value), and resulted into obsolete state. I've fixed it to return only the next state patch, and the concurrency issues with value updates are gone for good.

@kettanaito kettanaito force-pushed the 352-reactive-concurrency branch from 2075347 to 8e6be40 Compare February 3, 2019 13:52
@kettanaito kettanaito changed the base branch from master to 1.7 February 4, 2019 18:07
@kettanaito
Copy link
Owner Author

Although patched state updates eradicate field value concurrency issue that required to have a delay set while typing into inputs during tests, it still doesn't help with random integration tests failing during their CLI execution. That matter may require more investigation, and I don't plan to do that as a part of this pull request.

@kettanaito kettanaito changed the title WIP Introduce patched state update Introduce patched state update Feb 6, 2019
This was referenced Feb 7, 2019
@kettanaito
Copy link
Owner Author

I've tried to simplify record utils as a part of function call signature adjustment, and I can conclude that indeed a lot of payload coming into utility functions can be omitted, or replaced with the existing payload. I can't say it's perfect now, but some complicated utils (updateValidityState) where drastically simplified, removing the need for intermediate states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances existing functionality. refactoring scope:architecture Changes affect architecture.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant