Skip to content
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

Form.updateField: Use "update" function instead of "propsPatch" #182

Closed
kettanaito opened this issue Feb 12, 2018 · 3 comments
Closed

Form.updateField: Use "update" function instead of "propsPatch" #182

kettanaito opened this issue Feb 12, 2018 · 3 comments
Assignees
Labels

Comments

@kettanaito
Copy link
Owner

What

I suggest to at least try to change the behavior of Form.updateField to expose an updater function to update the requested field record.

Why

Right now propsPatch is an Object which gets converted to Immutable (1) and then merged (2) with the record via fieldRecord.merge(fromJS(propsPatch)). Those are two costly procedures and they should be avoided.

Exposing the requested fieldRecord inside a custom updater function would mean that that record can be modified right away, without conversion or merging. All the data needed for update is available to compose propsPatch, anyway.

How

this.updateField({
  fieldPath,
  updater: fieldRecord => fieldRecord
    .set('validatedSync', false)
    .set('validatedAsync', false) 
    .set('validSync', false)
    .set('validAsync', false)
});

Keynotes

  • See what will Form.updateField() do after the introduction of this change. Would it still be worthy to isolated into a separate method?
@kettanaito
Copy link
Owner Author

I wonder how this change would affect the dynamic props patch updates (i.e. for dynamic props change subscriptions).

With the current approach it's enough to just pass changedProps or similar object as the value of propsPatch. Once there is an updater function, what is the approach here? Merging can still be expensive, and assignment within the iteration will be as well.

@kettanaito kettanaito added the scope:architecture Changes affect architecture. label Feb 27, 2018
@kettanaito kettanaito self-assigned this Mar 31, 2018
@kettanaito
Copy link
Owner Author

I will try out this change and see how it integrates into existing solution.

@kettanaito kettanaito changed the title Form.updateField: Use "updater" function instead of "propsPatch" Form.updateField: Use "update" function instead of "propsPatch" Apr 2, 2018
@kettanaito
Copy link
Owner Author

Merged. Will be published in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant