[FormLayout] Converted to a functional component

[AndrewMusgrave]

WHY are these changes introduced?

More context here

This open in a tab, will polish and request reviews after hack days.

[github-actions]

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified	2
Files potentially affected	3

Details

All files potentially affected (total: 3)

• /src/components/ResourceList/components/FilterControl/components/FilterCreator/FilterCreator.tsx
• /src/components/ResourceList/components/FilterControl/FilterControl.tsx
• /src/components/ResourceList/ResourceList.tsx

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/FormLayout/FormLayout.tsx (total: 3)

Files potentially affected (total: 3)

• /src/components/ResourceList/components/FilterControl/components/FilterCreator/FilterCreator.tsx
• /src/components/ResourceList/components/FilterControl/FilterControl.tsx
• /src/components/ResourceList/ResourceList.tsx

---

This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

[BPScott]

Given my talk in #2062 about our cargo-culty usage of PureComponent and how I'd like us to stop doing that I don't think we should bother wrapping this in a React.memo

[dleroux]

This looks good pending the reply to Ben's comment. Was there a benefit here for using Memo?

[AndrewMusgrave]

@BPScott For this pull-request, I think I'm going to keep React.memo. It's a migration from a class to a functional component, and since FormLayout extends React.PureComponent we would be removing functionality.

However I'm not disagreeing that we probably don't need React.memo on FormLayout, the component is so simple a render is production would be negligible.

Before we start pulling React.PureComponent & React.Memo from our components we should probably come up with a decision log, outlining our choice and why so our patterns remain consistent.

As a devil's advocate, the render time of FormLayout may be negligible nonetheless using React.memo is negligible as well. Do we have benchmarks for the time blocked or saved by using React.Memo.

[BPScott]

Before we start pulling React.PureComponent & React.Memo from our components we should probably come up with a decision log, outlining our choice and why so our patterns remain consistent.

Excellent points :) I'm happy to say lets do one migration stage at a time, as long as we make sure we make space for doing the PureComponent audit at some point soon. I really like the idea of documenting this in a decision record. Tangentially I've been thinking about decision records for some other stuff too - how we handle imports/exports (i.e. why we avoid defaults), how we structure subcomponent and utility folders, opinions on how we name attributes (prefering positive namings execpt when defaults result in double negatives, like HTML attributes so disabled is fine) and how we handle event handlers (this is kinda up in the air - half our stuff has default noop functions, and half leaves them undefined and checks for the functions existence before calling them) - but they're all stories for another day :)

As a devil's advocate, the render time of FormLayout may be negligible nonetheless using React.memo is negligible as well. Do we have benchmarks for the time blocked or saved by using React.Memo.

Alas I know of no benchmarks - most stuff I've saw is very dependant on how the components are used - the more complex the props you pass in the longer the diff takes I guess. For honest meaningful figures we'd need to look at real world usage. Though my personal bias leans towards "stuff like FormLayout is so dang simple I'd bet on React.memo doing more harm than good" but I've got absolutely nothing to back that up.