fix: 'Oh snap' error when creating VPC from Linode Create

[coliu-akamai]

Description 📝

When the Linode Create v2 feature flag is not checked, trying to create a VPC from Linode Create results in the page crashing

Changes 🔄

• Update value field in VPC panel's autocomplete to ensure 'undefined' is never the value

Target release date 🗓️

8/14 - trying to get it into this upcoming release, as the bug it fixes is still unreleased!

Preview 📷

How to test 🧪

Reproduction steps

• Make sure Linode Create v2 flag is not checked
• Try creating a VPC from the Linode Create page

Verification steps

• Verify that creating a VPC from the Linode Create page does not crash the page when Linode Create v2 flag isn't checked
• Double check that when Linode Create v2 flag is checked, everything still works as expected

As an Author I have considered 🤔

Check all that apply

• 👀 Doing a self review
• ❔ Our contribution guidelines
• 🤏 Splitting feature into small PRs
• ➕ Adding a changeset
• 🧪 Providing/Improving test coverage
• 🔐 Removing all sensitive information from the code and PR description
• 🚩 Using a feature flag to protect the release
• 👣 Providing comprehensive reproduction steps
• 📑 Providing or updating our documentation
• 🕛 Scheduling a pair reviewing session
• 📱 Providing mobile support
• ♿ Providing accessibility support

---

Commit message and pull request title format standards

Note: Remove this section before opening the pull request
Make sure your PR title and commit message on squash and merge are as shown below

<commit type>: [JIRA-ticket-number] - <description>

Commit Types:

• feat: New feature for the user (not a part of the code, or ci, ...).
• fix: Bugfix for the user (not a fix to build something, ...).
• change: Modifying an existing visual UI instance. Such as a component or a feature.
• refactor: Restructuring existing code without changing its external behavior or visual UI. Typically to improve readability, maintainability, and performance.
• test: New tests or changes to existing tests. Does not change the production code.
• upcoming: A new feature that is in progress, not visible to users yet, and usually behind a feature flag.

Example: feat: [M3-1234] - Allow user to view their login history

---

[coliu-akamai]

Originally, I'd changed line 202 to be return option.label === value?.label;, since the bug is caused by the value being undefined. I thought that would be a super quick fix 😅 but this led to two MUI errors:

• controlled changing to uncontrolled input error
• "The value provided to Autocomplete is invalid. None of the options match with undefined."

ngl it took me a hot second to remember that array.find can return an undefined value oops 😅 so this change ensures that the item being passed to the value prop is always 'null' or some vpc dropdown option, not undefined. This fixes both the original problem and gets rid of the two MUI warnings above too!

This originally slipped past likely because the value prop is an optional parameter, so it can technically still be undefined (but we won't be passing it an undefined value).

[coliu-akamai]

(I've also been trying to play around with making it look cleaner so if anyone has suggestions lmk 😆)

[bnussman-akamai]

I see a few regressions still that we'll want to address

• We should probably add disableClearable
• We should probably remove the clearIcon prop
• We can probably remove isOptionEqualToValue

I'm thinking we can delete the defaultValue prop in favor of this for the value:

            value={
              selectedVPCId && selectedVPCId !== -1
                ? vpcDropdownOptions.find(
                    (option) => option.value === selectedVPCId
                  )
                : vpcDropdownOptions[0]
            

Linode Create v2 handles the "empty" state better by using a placeholder instead of a "None" option, but I think these steps will get Linode Create v1 back to what is use to be

[coliu-akamai]

👀 oh sweet thanks Banks! There does seem to be a slight change in initial behavior with the panel inside the Edit Config dialogue with those changes - the first VPC option gets selected instead of a default no option selected. Maybe this is fine though, since a person editing their Linode's configs to include a VPC wouldn't not select a VPC anyway?

local (also going to look into the styling change >> edit: seems more to do with autocomplete's styling, not from the VPC panel. Will still look into it on the side, but won't focus as much as it on this ticket)

prod

[coliu-akamai]

Maybe this is fine though, since a person editing their Linode's configs to include a VPC wouldn't not select a VPC anyway?

Looks like we do need to keep the default value for the Linode config, otherwise we run into the strange case where it seems we've selected a VPC, but can't select subnets for that VPC

I probably missing a few things but here were my thoughts on what I just pushed up:

• adding disableClearable prevented me from using null, which it seems we need for the Edit Config portion - using undefined led to controlled vs not controlled errors (?)
• essentially moved the value of defaultValue to be inside the value prop - so that initial values inside Edit Config and Linode Create should both now match prod, hopefully
• removed clearIcon and isOptionEqualToValue props
• there's a weird flicker when creating a VPC in the autocomplete compared to prod - I'm not too sure how to get rid of it tbh
• style regression not fixed 😢

I think my takeaway from this is that Linode Create v2 sounds really great 😅

[mjac0bs]

• adding disableClearable prevented me from using null, which it seems we need for the Edit Config portion - using undefined led to controlled vs not controlled errors (?)

This field was a point of trouble in the original PR too. Linking for context in case you hadn't read.

[coliu-akamai]

oh man, thanks for pointing that out @mjac0bs! I'm gonna add back in the clearIcon prop then + recheck everything

[github-actions]

Coverage Report: ✅
Base Coverage: 82.46%
Current Coverage: 82.46%

[hkhalil-akamai]

Great catch and thanks for addressing this promptly! Verified VPCPanel works as expected across the app.

[bnussman-akamai]

Looks good!

I still think clearIcon is a "hack" for not using disableClearable from a type safety perspective, but this seems to be working alright. It might require a bit more work in the future to dial this in