Color Palette Critical issue in 6.1

[mdmag]

Description

We were using ColorPalette component which we import from @wordpress/components
colors prop have been optional until 6.1 release
6.1 release crashes our app because it suppose that color prop is required.

Step-by-step reproduction instructions

1- Import ColorPalette from "@wordpress/components"
2- Don't pass colors prop
3- You will notice a javascript error

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[skorasaurus]

Hi,

Thanks for reporting; could you provide a code example of with the error occurring as well as the error message as well ; I am not able to reproduce any error on a test case where I have the following in my edit.js

import { ColorPalette } from "@wordpress/components";
import { useState } from "@wordpress/element";


const MyColorPalette = () => {
	const [color, setColor] = useState("#f00");

	return <ColorPalette value={color} onChange={(color) => setColor(color)} />;
};

[ndiego]

@ciampo I see you have done a decent amount of work on the ColorPalette component in the past few months. Any ideas on this one?

[Mamaduka]

Here's PR that aims to fix issue #45566.

I think we can use optional chaining as proposed in PR or set the default for colors params, as it's documented.

Cc @mirka

[mirka]

Ah, I think I understand what's going on now! The Gutenberg code in WP 6.1 is not up to date with our documentation, which is always in line with trunk.

In trunk, colors is documented as optional and it will not crash. In WP 6.1, colors is still required and will crash.

This change happened in dbbcc6e. So if you check the commit before that (6ee2a49), the docs say that colors is required, and will crash.

[Mamaduka]

@mirka, the missing required colors param wasn't crashing in 6.0 because we were using lodash.map, which won't trigger an error if a variable is undefined.

https://github.com/WordPress/gutenberg/blob/release/6.0/packages/components/src/color-palette/index.js#L29

Should we consider this as a breaking change?

I understand that it was documented as required, but people often ignore docs.

[griffbrad]

@tyxla This looks like a case where lodash was maybe being overly "forgiving" with types previously and covering up the issue. We should probably guard against this case to preserve the older behavior.

[tyxla]

Thanks for the ping! I'm going to take a look shortly.

[mirka]

Should we consider this as a breaking change?

Hard to say in this specific case, but definitely the type of change we should be careful of when removing lodash dependencies!

As for this particular issue, it has been back to the previous behavior since #44632, with docs updated to mark the prop as optional. It should also be guarded against future regressions of the sort, thanks to it being in TypeScript now.

So the immediate action item here would be to include #44632 in the next release (not sure if there's a cherry picking situation going on for the patch releases).

[Mamaduka]

Thank you, @mirka!

I will add #44632 to the WP 6.1.1 board and cherry-pick it for the minor release.

[tyxla]

From what I understand, there isn't anything else to be done here, other than cherry-picking #44632. Unless y'all think we need additional undefined handling?

[Mamaduka]

That's my understanding as well, @tyxla.

[Mamaduka]

The fix was backported into WP 6.1.1. I'll close the issue.