Skip to content

Commit

Permalink
fix(editor) Prevent invalid props being passed to fragments. (#5735)
Browse files Browse the repository at this point in the history
- Exclude properties other than `key` and `children` from fragments.
- Pass properties assigned to fragments down to their children.
  • Loading branch information
seanparsons authored May 24, 2024
1 parent e471d3f commit 657176a
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 11 deletions.
14 changes: 14 additions & 0 deletions editor/src/utils/canvas-react-utils.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,20 @@ describe('Monkey Function', () => {
`)
})

it('Props get pushed down off of a fragment onto the children', () => {
const ElementThatIsReallyAFragment: any = React.Fragment as any
expect(
renderToFormattedString(
<ElementThatIsReallyAFragment style={{ backgroundColor: 'red' }}>
<div>Hello!</div>
</ElementThatIsReallyAFragment>,
),
).toMatchInlineSnapshot(`
"<div style=\\"background-color: red\\">Hello!</div>
"
`)
})

it('Fragments work if theres a uid 2', () => {
const Component = () => {
return (
Expand Down
57 changes: 46 additions & 11 deletions editor/src/utils/canvas-react-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,12 @@ export function getDisplayName(type: any): string {
}
}

function isFragment(type: any): boolean {
return type === React.Fragment || type.$$typeof === fragmentSymbol
}

function fragmentOrProviderOrContext(type: any): boolean {
return (
type == React.Fragment ||
type?.$$typeof == fragmentSymbol ||
type?.$$typeof == providerSymbol ||
type?.$$typeof == contextSymbol
)
return isFragment(type) || type?.$$typeof == providerSymbol || type?.$$typeof == contextSymbol
}

function keyShouldBeExcluded(key: string): boolean {
Expand Down Expand Up @@ -204,6 +203,7 @@ const mangleExoticType = memoize(
child: React.ReactElement | null,
dataUid: string | null,
path: string | null,
fragmentParentProps: any,
) {
if (child == null || !shouldIncludeDataUID(child.type)) {
return child
Expand All @@ -218,6 +218,18 @@ const mangleExoticType = memoize(
// Setup the result.
let additionalProps: any = {}
let shouldClone: boolean = false
// For any properties that are not `key` or `children`, push those onto the children of the fragment.
// This has been done to handle some naughty behaviour seen where a library (in this case headlessui)
// sees our wrapper component and doesn't think it's a fragment (even though it's a wrapper around one) and
// then passes properties to the fragment instead of the children which is what it intends to do.
if (isFragment(type)) {
for (const parentPropsKey in fragmentParentProps) {
if (parentPropsKey !== 'key' && parentPropsKey !== 'children') {
additionalProps[parentPropsKey] = fragmentParentProps[parentPropsKey]
shouldClone = true
}
}
}

if (childUID != null) {
additionalProps[UTOPIA_UID_KEY] = childUID
Expand All @@ -242,9 +254,19 @@ const mangleExoticType = memoize(
* Instead of that we render these fragment-like components, and mangle with their children
*/
const wrapperComponent = (p: any, context?: any) => {
// Capture this early so that it's possible to prevent an early return.
let isFragmentWithNonFragmentProps: boolean = false
if (isFragment(type)) {
for (const parentPropsKey in p) {
if (parentPropsKey !== 'key' && parentPropsKey !== 'children') {
isFragmentWithNonFragmentProps = true
break
}
}
}
const uid = p?.[UTOPIA_UID_KEY]
const path = p?.[UTOPIA_PATH_KEY]
if (uid == null) {
if (uid == null && !isFragmentWithNonFragmentProps) {
// early return for the cases where there's no data-uid
return realCreateElement(type, p)
} else if (p?.children == null || typeof p.children === 'string') {
Expand All @@ -264,19 +286,32 @@ const mangleExoticType = memoize(

if (Array.isArray(p?.children)) {
children = React.Children.map(p?.children, (child) =>
updateChild(child, uidToPass, pathToPass),
updateChild(child, uidToPass, pathToPass, p),
)
} else {
children = updateChild(p.children, uidToPass, pathToPass)
children = updateChild(p.children, uidToPass, pathToPass, p)
}
}
let mangledProps = {
...p,
children: children,
}

delete mangledProps[UTOPIA_UID_KEY]
delete mangledProps[UTOPIA_PATH_KEY]
if (isFragment(type)) {
// Fragments cannot take any properties other than `key` or `children`,
// so we create a props object that only has those two properties.
let newMangledProps: any = {}
if (mangledProps.key !== undefined) {
newMangledProps.key = mangledProps.key
}
if (mangledProps.children !== undefined) {
newMangledProps.children = mangledProps.children
}
mangledProps = newMangledProps
} else {
delete mangledProps[UTOPIA_UID_KEY]
delete mangledProps[UTOPIA_PATH_KEY]
}
return realCreateElement(type as any, mangledProps)
}
}
Expand Down

0 comments on commit 657176a

Please sign in to comment.