Skip to content

Commit

Permalink
fix(editor) Fix deep equality checks for new expression types. (#4971)
Browse files Browse the repository at this point in the history 
- Renamed `JSXAttributeKeepDeepEqualityCall` to `JSExpressionKeepDeepEqualityCall`.
- Added `JSIdentifierKeepDeepEquality`, `JSPropertyAccessKeepDeepEquality` and
  `JSElementAccessKeepDeepEquality`.
- Added handling for the new expression types to `JSXElementChildKeepDeepEquality`.
- Added handling for the new expression types to `JSExpressionKeepDeepEqualityCall`.
- Change deep equality functions to exhaustive switch style.
  • Loading branch information
@seanparsons
seanparsons authored Feb 29, 2024
1 parent fe3c197 commit c59e60a
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 71 deletions.
8 changes: 4 additions & 4 deletions editor/src/components/editor/store/store-deep-equality-instances.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {
JSXArraySpreadKeepDeepEqualityCall,
JSXArrayValueKeepDeepEqualityCall,
JSXAttributeFunctionCallKeepDeepEqualityCall,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
JSXAttributeNestedArrayKeepDeepEqualityCall,
JSXAttributeNestedObjectKeepDeepEqualityCall,
JSExpressionOtherJavaScriptKeepDeepEqualityCall,
Expand Down Expand Up @@ -893,17 +893,17 @@ describe('JSXAttributeKeepDeepEqualityCall', () => {
const newDifferentValue = jsExpressionValue('new', emptyComments, 'new')

it('same reference returns the same reference', () => {
const result = JSXAttributeKeepDeepEqualityCall(oldValue, oldValue)
const result = JSExpressionKeepDeepEqualityCall(oldValue, oldValue)
expect(result.value).toBe(oldValue)
expect(result.areEqual).toEqual(true)
})
it('same value returns the same reference', () => {
const result = JSXAttributeKeepDeepEqualityCall(oldValue, newSameValue)
const result = JSExpressionKeepDeepEqualityCall(oldValue, newSameValue)
expect(result.value).toBe(oldValue)
expect(result.areEqual).toEqual(true)
})
it('different but similar value handled appropriately', () => {
const result = JSXAttributeKeepDeepEqualityCall(oldValue, newDifferentValue)
const result = JSExpressionKeepDeepEqualityCall(oldValue, newDifferentValue)
expect(result.value.type).toBe(oldValue.type)
expect((result.value as JSExpressionValue<string>).value).toBe(
(newDifferentValue as JSExpressionValue<string>).value,
Expand Down
251 changes: 184 additions & 67 deletions editor/src/components/editor/store/store-deep-equality-instances.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ import type {
JSXMapExpression,
JSExpressionMapOrOtherJavascript,
JSExpressionOtherJavaScript,
JSIdentifier,
JSPropertyAccess,
JSElementAccess,
} from '../../../core/shared/element-template'
import {
elementInstanceMetadata,
Expand Down Expand Up @@ -188,6 +191,16 @@ import {
isJSXMapExpression,
arbitraryJSBlock,
jsExpressionOtherJavaScript,
isJSExpressionValue,
isJSExpressionNestedArray,
isJSExpressionNestedObject,
isJSExpressionFunctionCall,
jsIdentifier,
jsPropertyAccess,
jsElementAccess,
isJSIdentifier,
isJSPropertyAccess,
isJSElementAccess,
} from '../../../core/shared/element-template'
import type {
CanvasRectangle,
Expand Down Expand Up @@ -982,7 +995,7 @@ export const JSExpressionOtherJavaScriptOrJSXMapExpressionKeepDeepEqualityCall:
export function JSXArrayValueKeepDeepEqualityCall(): KeepDeepEqualityCall<JSXArrayValue> {
return combine2EqualityCalls(
(value) => value.value,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(value) => value.comments,
ParsedCommentsKeepDeepEqualityCall,
jsxArrayValue,
Expand All @@ -992,7 +1005,7 @@ export function JSXArrayValueKeepDeepEqualityCall(): KeepDeepEqualityCall<JSXArr
export function JSXArraySpreadKeepDeepEqualityCall(): KeepDeepEqualityCall<JSXArraySpread> {
return combine2EqualityCalls(
(value) => value.value,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(value) => value.comments,
ParsedCommentsKeepDeepEqualityCall,
jsxArraySpread,
Expand Down Expand Up @@ -1026,7 +1039,7 @@ export function JSXAttributeNestedArrayKeepDeepEqualityCall(): KeepDeepEqualityC
export function JSXSpreadAssignmentKeepDeepEqualityCall(): KeepDeepEqualityCall<JSXSpreadAssignment> {
return combine2EqualityCalls(
(value) => value.value,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(value) => value.comments,
ParsedCommentsKeepDeepEqualityCall,
jsxSpreadAssignment,
Expand All @@ -1038,7 +1051,7 @@ export function JSXPropertyAssignmentKeepDeepEqualityCall(): KeepDeepEqualityCal
(value) => value.key,
createCallWithTripleEquals(),
(value) => value.value,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(value) => value.comments,
ParsedCommentsKeepDeepEqualityCall,
(value) => value.keyComments,
Expand Down Expand Up @@ -1076,50 +1089,76 @@ export function JSXAttributeFunctionCallKeepDeepEqualityCall(): KeepDeepEquality
(value) => value.functionName,
createCallWithTripleEquals(),
(value) => value.parameters,
arrayDeepEquality(JSXAttributeKeepDeepEqualityCall),
arrayDeepEquality(JSExpressionKeepDeepEqualityCall),
(value) => value.uid,
createCallWithTripleEquals<string>(),
jsExpressionFunctionCall,
)
}

export const JSXAttributeKeepDeepEqualityCall: KeepDeepEqualityCall<JSExpression> = (
export const JSExpressionKeepDeepEqualityCall: KeepDeepEqualityCall<JSExpression> = (
oldAttribute,
newAttribute,
) => {
if (isJSXAttributeValue(oldAttribute) && isJSXAttributeValue(newAttribute)) {
return JSXAttributeValueKeepDeepEqualityCall(oldAttribute, newAttribute)
} else if (
modifiableAttributeIsAttributeOtherJavaScript(oldAttribute) &&
modifiableAttributeIsAttributeOtherJavaScript(newAttribute)
) {
return JSExpressionOtherJavaScriptKeepDeepEqualityCall()(oldAttribute, newAttribute)
} else if (
modifiableAttributeIsAttributeNestedArray(oldAttribute) &&
modifiableAttributeIsAttributeNestedArray(newAttribute)
) {
return JSXAttributeNestedArrayKeepDeepEqualityCall()(oldAttribute, newAttribute)
} else if (
modifiableAttributeIsAttributeNestedObject(oldAttribute) &&
modifiableAttributeIsAttributeNestedObject(newAttribute)
) {
return JSXAttributeNestedObjectKeepDeepEqualityCall()(oldAttribute, newAttribute)
} else if (
modifiableAttributeIsAttributeFunctionCall(oldAttribute) &&
modifiableAttributeIsAttributeFunctionCall(newAttribute)
) {
return JSXAttributeFunctionCallKeepDeepEqualityCall()(oldAttribute, newAttribute)
} else {
return keepDeepEqualityResult(newAttribute, false)
switch (oldAttribute.type) {
case 'JSX_MAP_EXPRESSION':
if (isJSXMapExpression(newAttribute)) {
return JSXMapExpressionKeepDeepEqualityCall()(oldAttribute, newAttribute)
}
break
case 'ATTRIBUTE_VALUE':
if (isJSExpressionValue(newAttribute)) {
return JSXAttributeValueKeepDeepEqualityCall(oldAttribute, newAttribute)
}
break
case 'JS_IDENTIFIER':
if (isJSIdentifier(newAttribute)) {
return JSIdentifierKeepDeepEquality()(oldAttribute, newAttribute)
}
break
case 'JS_ELEMENT_ACCESS':
if (isJSElementAccess(newAttribute)) {
return JSElementAccessKeepDeepEquality()(oldAttribute, newAttribute)
}
break
case 'JS_PROPERTY_ACCESS':
if (isJSPropertyAccess(newAttribute)) {
return JSPropertyAccessKeepDeepEquality()(oldAttribute, newAttribute)
}
break
case 'ATTRIBUTE_NESTED_ARRAY':
if (isJSExpressionNestedArray(newAttribute)) {
return JSXAttributeNestedArrayKeepDeepEqualityCall()(oldAttribute, newAttribute)
}
break
case 'ATTRIBUTE_NESTED_OBJECT':
if (isJSExpressionNestedObject(newAttribute)) {
return JSXAttributeNestedObjectKeepDeepEqualityCall()(oldAttribute, newAttribute)
}
break
case 'ATTRIBUTE_FUNCTION_CALL':
if (isJSExpressionFunctionCall(newAttribute)) {
return JSXAttributeFunctionCallKeepDeepEqualityCall()(oldAttribute, newAttribute)
}
break
case 'ATTRIBUTE_OTHER_JAVASCRIPT':
if (isJSExpressionOtherJavaScript(newAttribute)) {
return JSExpressionOtherJavaScriptKeepDeepEqualityCall()(oldAttribute, newAttribute)
}
break
default:
assertNever(oldAttribute)
}

return keepDeepEqualityResult(newAttribute, false)
}

export function JSXAttributesEntryDeepEqualityCall(): KeepDeepEqualityCall<JSXAttributesEntry> {
return combine3EqualityCalls(
(entry) => entry.key,
createCallWithTripleEquals(),
(entry) => entry.value,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(entry) => entry.comments,
ParsedCommentsKeepDeepEqualityCall,
jsxAttributesEntry,
Expand All @@ -1129,7 +1168,7 @@ export function JSXAttributesEntryDeepEqualityCall(): KeepDeepEqualityCall<JSXAt
export function JSXAttributesSpreadDeepEqualityCall(): KeepDeepEqualityCall<JSXAttributesSpread> {
return combine2EqualityCalls(
(entry) => entry.spreadValue,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(entry) => entry.comments,
ParsedCommentsKeepDeepEqualityCall,
jsxAttributesSpread,
Expand Down Expand Up @@ -1205,41 +1244,119 @@ export function ArbitraryJSBlockKeepDeepEquality(): KeepDeepEqualityCall<Arbitra
)
}

export function JSIdentifierKeepDeepEquality(): KeepDeepEqualityCall<JSIdentifier> {
return combine3EqualityCalls(
(identifier) => identifier.name,
createCallWithTripleEquals<string>(),
(identifier) => identifier.uid,
createCallWithTripleEquals<string>(),
(identifier) => identifier.comments,
ParsedCommentsKeepDeepEqualityCall,
jsIdentifier,
)
}

export function JSPropertyAccessKeepDeepEquality(): KeepDeepEqualityCall<JSPropertyAccess> {
return combine4EqualityCalls(
(access) => access.onValue,
JSExpressionKeepDeepEqualityCall,
(access) => access.property,
createCallWithTripleEquals<string>(),
(access) => access.uid,
createCallWithTripleEquals<string>(),
(access) => access.comments,
ParsedCommentsKeepDeepEqualityCall,
jsPropertyAccess,
)
}

export function JSElementAccessKeepDeepEquality(): KeepDeepEqualityCall<JSElementAccess> {
return combine4EqualityCalls(
(access) => access.onValue,
JSExpressionKeepDeepEqualityCall,
(access) => access.element,
JSExpressionKeepDeepEqualityCall,
(access) => access.uid,
createCallWithTripleEquals<string>(),
(access) => access.comments,
ParsedCommentsKeepDeepEqualityCall,
jsElementAccess,
)
}

export function JSXElementChildKeepDeepEquality(): KeepDeepEqualityCall<JSXElementChild> {
return (oldElement, newElement) => {
if (isJSXElement(oldElement) && isJSXElement(newElement)) {
return JSXElementKeepDeepEquality(oldElement, newElement)
} else if (isJSXTextBlock(oldElement) && isJSXTextBlock(newElement)) {
return JSXTextBlockKeepDeepEquality(oldElement, newElement)
} else if (isJSXFragment(oldElement) && isJSXFragment(newElement)) {
return JSXFragmentKeepDeepEquality(oldElement, newElement)
} else if (isJSXConditionalExpression(oldElement) && isJSXConditionalExpression(newElement)) {
return JSXConditionalExpressionKeepDeepEquality(oldElement, newElement)
} else if (oldElement.type === 'ATTRIBUTE_VALUE' && newElement.type === 'ATTRIBUTE_VALUE') {
return JSXAttributeValueKeepDeepEqualityCall(oldElement, newElement)
} else if (
oldElement.type === 'ATTRIBUTE_OTHER_JAVASCRIPT' &&
newElement.type === 'ATTRIBUTE_OTHER_JAVASCRIPT'
) {
return JSExpressionOtherJavaScriptKeepDeepEqualityCall()(oldElement, newElement)
} else if (
oldElement.type === 'ATTRIBUTE_NESTED_ARRAY' &&
newElement.type === 'ATTRIBUTE_NESTED_ARRAY'
) {
return JSXAttributeNestedArrayKeepDeepEqualityCall()(oldElement, newElement)
} else if (
oldElement.type === 'ATTRIBUTE_NESTED_OBJECT' &&
newElement.type === 'ATTRIBUTE_NESTED_OBJECT'
) {
return JSXAttributeNestedObjectKeepDeepEqualityCall()(oldElement, newElement)
} else if (
oldElement.type === 'ATTRIBUTE_FUNCTION_CALL' &&
newElement.type === 'ATTRIBUTE_FUNCTION_CALL'
) {
return JSXAttributeFunctionCallKeepDeepEqualityCall()(oldElement, newElement)
} else {
return keepDeepEqualityResult(newElement, false)
switch (oldElement.type) {
case 'JSX_ELEMENT':
if (isJSXElement(newElement)) {
return JSXElementKeepDeepEquality(oldElement, newElement)
}
break
case 'JSX_FRAGMENT':
if (isJSXFragment(newElement)) {
return JSXFragmentKeepDeepEquality(oldElement, newElement)
}
break
case 'JSX_TEXT_BLOCK':
if (isJSXTextBlock(newElement)) {
return JSXTextBlockKeepDeepEquality(oldElement, newElement)
}
break
case 'JSX_CONDITIONAL_EXPRESSION':
if (isJSXConditionalExpression(newElement)) {
return JSXConditionalExpressionKeepDeepEquality(oldElement, newElement)
}
break
case 'JSX_MAP_EXPRESSION':
if (isJSXMapExpression(newElement)) {
return JSXMapExpressionKeepDeepEqualityCall()(oldElement, newElement)
}
break
case 'ATTRIBUTE_VALUE':
if (isJSExpressionValue(newElement)) {
return JSXAttributeValueKeepDeepEqualityCall(oldElement, newElement)
}
break
case 'JS_IDENTIFIER':
if (isJSIdentifier(newElement)) {
return JSIdentifierKeepDeepEquality()(oldElement, newElement)
}
break
case 'JS_ELEMENT_ACCESS':
if (isJSElementAccess(newElement)) {
return JSElementAccessKeepDeepEquality()(oldElement, newElement)
}
break
case 'JS_PROPERTY_ACCESS':
if (isJSPropertyAccess(newElement)) {
return JSPropertyAccessKeepDeepEquality()(oldElement, newElement)
}
break
case 'ATTRIBUTE_NESTED_ARRAY':
if (isJSExpressionNestedArray(newElement)) {
return JSXAttributeNestedArrayKeepDeepEqualityCall()(oldElement, newElement)
}
break
case 'ATTRIBUTE_NESTED_OBJECT':
if (isJSExpressionNestedObject(newElement)) {
return JSXAttributeNestedObjectKeepDeepEqualityCall()(oldElement, newElement)
}
break
case 'ATTRIBUTE_FUNCTION_CALL':
if (isJSExpressionFunctionCall(newElement)) {
return JSXAttributeFunctionCallKeepDeepEqualityCall()(oldElement, newElement)
}
break
case 'ATTRIBUTE_OTHER_JAVASCRIPT':
if (isJSExpressionOtherJavaScript(newElement)) {
return JSExpressionOtherJavaScriptKeepDeepEqualityCall()(oldElement, newElement)
}
break
default:
assertNever(oldElement)
}

return keepDeepEqualityResult(newElement, false)
}
}

Expand Down Expand Up @@ -1355,7 +1472,7 @@ export const JSXConditionalExpressionKeepDeepEquality: KeepDeepEqualityCall<JSXC
(conditional) => conditional.uid,
StringKeepDeepEquality,
(conditional) => conditional.condition,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
(conditional) => conditional.originalConditionString,
StringKeepDeepEquality,
(conditional) => conditional.whenTrue,
Expand Down Expand Up @@ -1970,7 +2087,7 @@ export const EditorStateCanvasTransientPropertyKeepDeepEquality: KeepDeepEqualit
(property) => property.elementPath,
ElementPathKeepDeepEquality,
(property) => property.attributesToUpdate,
objectDeepEquality(JSXAttributeKeepDeepEqualityCall),
objectDeepEquality(JSExpressionKeepDeepEqualityCall),
editorStateCanvasTransientProperty,
)

Expand Down Expand Up @@ -4031,7 +4148,7 @@ export const ValueAtPathDeepEquality: KeepDeepEqualityCall<ValueAtPath> = combin
(c) => c.path,
PropertyPathKeepDeepEquality(),
(c) => c.value,
JSXAttributeKeepDeepEqualityCall,
JSExpressionKeepDeepEqualityCall,
valueAtPath,
)

Expand Down

0 comments on commit c59e60a

Please sign in to comment.