Custom components with noWrap cause console warning regarding key

Bugs
1

Detailed steps to reproduce the bug

  1. Create a custom component and set noWrap: true
Builder.registerComponent(Component, { 	
  name: 'VerticalSpacer',
  noWrap: true,
  ...
});
  1. Pass attributes to component
export const VerticalSpacer: React.FC<VerticalSpacerProps> = ({ attributes }) => {
  const { className: builderClassNames, ...restOfAttributes } = attributes || {}; 
  
return <div className={classNames(styles.spacer, builderClassNames)} {...restOfAttributes} />; 
};
  1. Now the console will complain about A props object containing a "key" prop is being spread into JSX.

Screenshots or video link

Screenshot 2024-10-10 at 06.30.49
Screenshot 2024-10-10 at 06.30.491208Ă—256 209 KB

Code stack you are integrating Builder with
React, Typescript

Versions in package.json:
“@builder.io/react”: “4.0.0”,
“@builder.io/sdk”: “2.2.6”,

1 Like
2

Hi Krisztina​,

Thank you for contacting Builder.io Support! I look forward to helping you with your request.

Are you using {...props.attributes}?

Please check this documentation Options for registering custom components - Builder.io for more details.

Thanks,

3

Yes, restOfAttributes is the destructured props.attributes besides className.

As you can see {...restOfAttributes} are being passed to the outermost div being returned:

return <div className={classNames(styles.spacer, builderClassNames)} {...restOfAttributes} />;

We have had this custom component for at least 8-9 months, but i haven’t seen the warning in the console before. So i am not sure if checks for how the key prop is being passed to elements has gotten stricter somehow…?

4

Hello Krisztina,

Could you share the custom component code?

See if I can replicate the error from my end.

Thank you,

5

Sure, here you go:

VericalSpacer.tsx:

interface VerticalSpacerProps extends VerticalSpacerComponentProps<'unset' | SpacingSize['value']> {
	readonly attributes?: any;
}

export const VerticalSpacer: React.FunctionComponent<VerticalSpacerProps> = ({
	size,
	attributes,
}) => {
	const { className: builderClassNames, ...restOfAttributes } = attributes || {};

	return (
		<div
			className={classNames(
				styles.wrapper,
				builderClassNames,
			)}
			{...restOfAttributes}
			style={
				{
					'--size': size,
				} as React.CSSProperties
			}
		/>
	);
};

register.ts:

Builder.registerComponent(Component, {
	name: 'VerticalSpacer',
	noWrap: true,
	inputs: [
		{
			name: 'size',
			type: 'string',
			required: true,
			enum: ['unset', 'sm', 'md', 'lg', 'xl'],
			defaultValue: 'sm',
		}
	],
});
6

Hello Krisztina,

We have been trying to replicate this issue, but your code has dependencies that we do not have.

Could you share the custom component code and the dependencies?

That way, we can replicate the issue and try to find a solution faster.

Thank you,

7

I have assumed it was a general issue with custom components and noWrap that get passed attributes as prop. Is that not the case? I would not expect there is anything in my code that is causing it.

Is that not the case, can you not reproduce it yourself?

8

Hello Kriszta,

Thank you for your reply.

We have been trying to replicate the issue without success.

For the same reason, we think it could be related to the custom code.

If you do not want to post your code here, is there a way you could create a test space with the issue present?

Thank you,

9

You should be able to reproduce it in development mode. I think the react warnings get disabled unless it’s a dev build.
I have just used a barebone project from your docs site and managed to reproduce the issue: Developer Quickstart - Builder.io

Just by creating a custom component with noWrap: true and spreading attributes to the outermost container, like recommended in your docs.

The issue is that a “key” is being spread into attributes without a need for it.

For now i have solved it by setting the key to something hardcoded manually after spreading the attributes (see code example below), but the core issue remains:
Attributes received from builder should not contain a key, or your docs needs to be updated regarding how to use noWrap: true with custom components.

<div
	className={classNames(
		styles.wrapper,
		builderClassNames,
	)}
	{...restOfAttributes}
	key={'verticalSpacer'}
/>
10

Hello @kriszta ,

Thank you for your detailed feedback regarding the issue with the key attribute being included in the props for custom components using noWrap: true. I appreciate your effort in reproducing the issue with a barebone project from our documentation.

You are correct that React warnings might be disabled unless in development mode, and it’s important that our documentation accurately reflects the behavior of custom components. The core issue of the key being included in the attributes without necessity is noted.

To resolve this in your implementation, you can ensure that the key is assigned directly in your JSX while still utilizing the other properties from the object. Here’s an updated version of your component that accommodates this structure:

interface VerticalSpacerProps {
  size: 'unset' | 'small' | 'medium' | 'large';
  attributes?: Omit<React.HTMLAttributes<HTMLDivElement>, 'key'> & { key?: React.Key }; // Allow key here
  className?: string; // For additional custom classes
}

export const VerticalSpacer: React.FC<VerticalSpacerProps> = ({ size, attributes, className }) => {
  const { className: builderClassNames, key, ...restOfAttributes } = attributes || {};

  return (
    <div
      key={key} // Pass key directly
      className={`${builderClassNames || ''} ${className || ''}`.trim()}
      {...restOfAttributes} // Spread the remaining attributes
      style={{ '--size': size, ...restOfAttributes?.style } as React.CSSProperties}
    />
  );
};

We will work with our documentation team to clarify the usage of noWrap: true with custom components to prevent confusion in the future.

Thank you for bringing this to our attention, and please let me know if you have any further questions or suggestions!

Thanks,

11

Thanks Manish, sounds good regarding you guys updating the docs.

However, regarding your suggestion above: i don’t see the need to pass the key from attributes on this <div> element. Keys are used to hold order in lists. And as far as i can tell, this is not a list, so this element does not need a unique key among its siblings. Therefore we might as well hardcode the key. Am I seeing it wrong?

12

Hello @kriszta,

You’re correct in identifying that the key attribute is typically used in lists to help React identify which items have changed, are added, or are removed. In the context of a single <div> element, independently of a list, there’s generally no need to pass the key from attributes.

The key is best suited for elements that are part of an array where the order and identity might change, so in your case, if VerticalSpacer is not part of an array rendered by map or similar methods, there’s no need to pass a dynamic key.

1 Like