Allowing generic item type in new experimental SelectControl

[joelclimbsthings]

• Have you followed the WooCommerce Contributing guideline?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Updates to the new experimental SelectControl to support passing a custom type to describe the items, as well as including getItemValue() and getItemLabel() functions as props to parse those items.

Note: This PR was a bit adventurous for me as I wasn't entirely familiar with TS generic types, but it was a fun learning experience. The only issue I haven't been able to resolve is a couple TS errors within the utils for the default item and value getters.

Closes #34399 .

How to test the changes in this Pull Request:

1. Checkout branch.
2. Fire up storybook with npm run storybook
3. Observe and interact with the new Experimental > SelectControl > Custom Item Type story.
4. Optionally including the Select control with a custom type (can copy from story) live on any react screen to interact with it there.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully run tests with your changes locally?
• Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

FOR PR REVIEWER ONLY:

• I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

[joelclimbsthings]

Referencing item.label and item.value below are causing TS errors, since TS isn't aware that they're only intended to be used along with the default item type. Is there any way to remedy this in a TS supported way?

[louwie17]

Yeah, you can make us of the is type guard -> https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards
The easiest way to do this is by creating a isDefaultItemType function, like so:

function isDefaultItemType(item: ItemType | null): item is ItemType {
  return (item as ItemType).label !== undefined;
}

You could include the check for value here as well if you wanted.
Then in the above code you can do:

if ( isDefaultItemType(item) ) {
  return item.label;
} else {
return '';
}

TypeScript should not complain about item.label within the if statement now.

[github-actions]

Test Results Summary

Commit SHA: b8b9ef7

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	118	0	0	2	0	120	0m 49s
E2E Tests	186	0	0	1	0	187	14m 54s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

[louwie17]

Really nice work on this @joelclimbsthings, this is testing really well, I merged it into my category field branch locally to try the changes out and things tested well out of the box.

I only have one suggestion for the getItemValueType return value to allow for number as well, as a lot of id's are numbers.
And I left a response to your TS question here: https://github.com/woocommerce/woocommerce/pull/34547/files#r960866165

[louwie17]

Yeah, you can make us of the is type guard -> https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards
The easiest way to do this is by creating a isDefaultItemType function, like so:

function isDefaultItemType(item: ItemType | null): item is ItemType {
  return (item as ItemType).label !== undefined;
}

You could include the check for value here as well if you wanted.
Then in the above code you can do:

if ( isDefaultItemType(item) ) {
  return item.label;
} else {
return '';
}

TypeScript should not complain about item.label within the if statement now.

[louwie17]

Could we also allow the return of a number -> string | number for the value type? This is not a must if it will require a bunch of checks and conversions everywhere else, but if we can do it somewhat seamless then that would be really nice.

[joelclimbsthings]

Thanks for the help @louwie17 ! I believe I remedied the above, so let me know what you think. 👂🏻

[louwie17]

Thanks for the updates @joelclimbsthings, this tested well and the code looks great, very nice work!!

[louwie17]

@joelclimbsthings it does look like your changelog Github action failed with the same error that Josh ran into ( p1663259087459849-slack-C03CPM3UXDJ ), I think it just required a rebase to fix. Let me know if you need a re-approval.

[louwie17]

Re-approving after rebase, LGTM 🚀

[github-actions]

Hi @joelclimbsthings, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

• Add the release: add testing instructions label