[Form lib] Export internal state instead of raw state #80842

sebelga · 2020-10-16T15:17:19Z

Read the issue for context: #78589

This PR replaces the raw (flat object of value) state exposed to the consumer and replaces it with the form internal state (which default to the form "out" interface if no deserializer was provided).

interface MyForm {
  title: string;
  subTitle: string;
}

// Internally we might need to add some field for our UI
interface MyFormInternal extends MyForm {
  _meta: {
    showAdvancedSettings: boolean; // additional field on the form, only for the UI
  }
}

// We provide both interface to the useForm call
const { form } = useForm<MyForm, MyFormInternal>();

// And we can provide them to useFormData().
// We now receive the internal interface providedinstead of the raw flat object
const [formData, serializer] = useFormData<MyFormInternal, MyForm>({ form });

formData // MyFormInternal
const output = serializer(); // MyForm


// If we don't use (de)serializer, then we can pass the "out" interface
const [formData] = useFormData<MyForm>({ form });

Fixes #78589

cc @rylnd @patrykkopycinski

elasticmachine · 2020-10-19T16:51:55Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

…aw-state

patrykkopycinski

That looks Amazing, Thank you @sebelga for all the hard work! 💪
SIEM/Endpoint LGTM! 🚀

jloleysens

Great work @sebelga ! I tested proposed changes locally in ingest pipelines plugin and in index lifecycle management.

Left a couple of comments.

jloleysens · 2020-10-20T09:00:38Z

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts

+      ? serializer(unflattenObject<I>(fieldsValue))
+      : unflattenObject<T>(fieldsValue);


So the form lib, internally still stores paths as my.path[0].to.something? We just don't expose this to consumer code?

Yes. I thought in changing that but when I started to write the logic to remove a field from an array (e.g. second position) from inside an object (at any depth) I thought... let's keep it flat until the last minute! 😄 It makes the internal logic much simpler.

jloleysens · 2020-10-20T09:02:28Z

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts

      if (resetValues) {
-        getFormData$().next(currentFormData as T);
+        updateFormData$(currentFormData);


Very nice update! RIP as!

jloleysens · 2020-10-20T09:04:25Z

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form_data.ts

+  /**
+   * We do want to offer to the consumer a handler to serialize the form data that changes each time
+   * the formData **state** changes. This is why we added the "formData" dep to the array and added the eslint override.
+   */
+  const serializer = useCallback(() => {
+    return getFormData();
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [getFormData, formData]);


👍 Happy for the form lib to guarantee this for consumers

kibanamachine · 2020-10-20T09:35:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 0917297

Metrics [docs]

async chunks size

id	before	after	diff
`indexLifecycleManagement`	268.7KB	269.0KB	+332.0B
`indexManagement`	1.6MB	1.6MB	+774.0B
`ingestPipelines`	813.0KB	813.1KB	+20.0B
`securitySolution`	8.1MB	8.1MB	+146.0B
total			+1.2KB

page load bundle size

id	before	after	diff
`esUiShared`	304.2KB	303.2KB	-1016.0B
`ingestPipelines`	42.0KB	41.9KB	-99.0B
total			-1.1KB

History

💔 Build #82510 failed 364855c
💔 Build #82465 failed f5edadc34964a45c2776aa5dcaf420bdace769de
💔 Build #82191 failed b0339592d13ec1f3dcfd9b6addbc52449ed92a4e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

sebelga · 2020-10-20T11:50:19Z

Thanks for the review @jloleysens !

…81143)

* master: (64 commits) Rename Security Solution Bug Template (elastic#81187) Update links (elastic#81125) Specify format for date range query (elastic#81025) [Alerting] Improve toast when alert is created (elastic#80327) [UX] Add empty states (elastic#80904) Add TS config for kibana_legacy (elastic#80992) [Telemetry] Add method to enable endpoint security data usage example (elastic#80940) [Alerting] Add scoped cluster client to alerts and actions services (elastic#80794) Fix reactRouterNavigate when used with a string (elastic#80520) [Security Solution] [Detections] Read privileges for dependencies (elastic#80852) [ML] Fixing exclude frequent in advanced wizard (elastic#81121) Fix security solution template label (elastic#80976) [DOCS] Update index management docs (elastic#80893) [APM] Error rate on service list page is not in sync with the value at the transaction page (elastic#80814) skip flaky suite (elastic#81072) [Task Manager] Cleans up legacy plugin structure (elastic#80381) Support unsigned_long fields (elastic#81115) [Form lib] Export internal state instead of raw state (elastic#80842) [Lens] Add toast notification when visualization is saved (elastic#80788) Index pattern edit field formatter API (elastic#78352) ...

sebelga added release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.1.1 v8.0.0 labels Oct 16, 2020

sebelga force-pushed the form-lib/remove-raw-state branch from b033959 to f5edadc Compare October 19, 2020 14:44

sebelga added 18 commits October 19, 2020 18:42

[Form lib] Refactor "errorDisplayDelay" --> "valueChangeDebounceTime"

b918471

[Form lib] Don't expose "getFieldDefaultValue" in the form API

8bb0890

[Form lib] Add FormHook TS docs

36d546f

[Form lib] Don't set the validity to undefined before validating

3df3ad3

Fix TS issues

927f800

Update the lib to work with internal **unflatten** object.

70f1865

Unflatten internal state only in useFormData and subscribe()

d8ddb51

Fix tests

d0fd4ab

Update uses of FormDataProvider

2dedeb5

Update uses of useFormData

4c84272

useFormData() : update the serializer handler along with the state

38555ef

Remove unnecessary useFormData() from pipeline documents tab

d163508

Remove unnecessary Partial in type

6ccef56

Fix scaling_factor additional field

daa2882

Fix date_range additional field

2e6d167

Fix date_range additional field (2)

260ded3

Fix TS issues

24f0e4a

Update ILM form

364855c

sebelga force-pushed the form-lib/remove-raw-state branch from 48ed86c to 364855c Compare October 19, 2020 16:51

sebelga requested a review from jloleysens October 19, 2020 16:51

sebelga marked this pull request as ready for review October 19, 2020 16:51

sebelga requested review from a team as code owners October 19, 2020 16:51

sebelga added 2 commits October 20, 2020 09:53

Fix TS issue

91260b6

Merge remote-tracking branch 'upstream/master' into form-lib/remove-r…

0917297

…aw-state

patrykkopycinski approved these changes Oct 20, 2020

View reviewed changes

jloleysens approved these changes Oct 20, 2020

View reviewed changes

sebelga merged commit 702e0c7 into elastic:master Oct 20, 2020

sebelga deleted the form-lib/remove-raw-state branch October 20, 2020 11:51

This was referenced Oct 20, 2020

[7.x] [Form lib] Export internal state instead of raw state (#80842) #81143

Merged

[Form lib] Get ready for v1.0.0 #65654

Open

sebelga added a commit that referenced this pull request Oct 20, 2020

[7.x] [Form lib] Export internal state instead of raw state (#80842) (#…

80d3349

…81143)

rylnd mentioned this pull request Oct 20, 2020

[SIEM] Narrow our Form types when using hook_form_lib #60567

Closed

sebelga mentioned this pull request Oct 21, 2020

[Mappings editor] Fix regressions in scaled_float and date_range types #81286

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form lib] Export internal state instead of raw state #80842

[Form lib] Export internal state instead of raw state #80842

sebelga commented Oct 16, 2020 •

edited

Loading

elasticmachine commented Oct 19, 2020

patrykkopycinski left a comment

jloleysens left a comment

jloleysens Oct 20, 2020

sebelga Oct 20, 2020

jloleysens Oct 20, 2020

jloleysens Oct 20, 2020

kibanamachine commented Oct 20, 2020

sebelga commented Oct 20, 2020

		? serializer(unflattenObject<I>(fieldsValue))
		: unflattenObject<T>(fieldsValue);

[Form lib] Export internal state instead of raw state #80842

[Form lib] Export internal state instead of raw state #80842

Conversation

sebelga commented Oct 16, 2020 • edited Loading

elasticmachine commented Oct 19, 2020

patrykkopycinski left a comment

Choose a reason for hiding this comment

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Oct 20, 2020

Choose a reason for hiding this comment

sebelga Oct 20, 2020

Choose a reason for hiding this comment

jloleysens Oct 20, 2020

Choose a reason for hiding this comment

jloleysens Oct 20, 2020

Choose a reason for hiding this comment

kibanamachine commented Oct 20, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

page load bundle size

History

sebelga commented Oct 20, 2020

sebelga commented Oct 16, 2020 •

edited

Loading