Skip to content

Commit

Permalink
fix(editor) Handle optional chaining of expressions. (#4992)
Browse files Browse the repository at this point in the history
- Added `optionallyChained` field to `JSPropertyAccess` and `JSElementAccess`.
- Updated `jsxAttributeToValue` to handle optionally chained versions of the
  two expression types that support them.
- `createJSElementAccess` and `createJSPropertyAccess` now check to see if
  the expression has the `questionDotToken` property and set `optionallyChained`
  appropriately, with the old handling for this property being removed.
- Updated `jsxAttributeToExpression` to add in the `QuestionDotToken` token
  if `optionallyChained` is set.
- Fixed an issue with `outermostWrapInBraces` where it should only wrap a value
  if the `expressionContext` value is `jsx`.
- Added `jsxAttributeToValue` tests for identifier, property and element accesses.
- Refactored out some exception handling into an outer function
  for `jsxAttributeToValue`.
- Added `resultOrError` utility function.
  • Loading branch information
seanparsons authored Mar 6, 2024
1 parent 44c39a4 commit d9411e8
Show file tree
Hide file tree
Showing 13 changed files with 805 additions and 145 deletions.
150 changes: 100 additions & 50 deletions editor/src/components/canvas/__snapshots__/ui-jsx-canvas.spec.tsx.snap

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,318 @@
import { isLeft } from '../../../core/shared/either'
import type { JSElementAccess } from '../../../core/shared/element-template'
import {
emptyComments,
isJSElementAccess,
isJSXElement,
isUtopiaJSXComponent,
jsElementAccess,
jsIdentifier,
jsPropertyAccess,
} from '../../../core/shared/element-template'
import { fromField, fromTypeGuard, traverseArray } from '../../../core/shared/optics/optic-creators'
import { toFirst } from '../../../core/shared/optics/optic-utilities'
import type { Optic } from '../../../core/shared/optics/optics'
import type { ParsedTextFile } from '../../../core/shared/project-file-types'
import { isParseSuccess } from '../../../core/shared/project-file-types'
import { emptySet } from '../../../core/shared/set-utils'
import { lintAndParse } from '../../../core/workers/parser-printer/parser-printer'
import { jsxElementChildToText } from './jsx-element-child-to-text'

describe('jsxElementChildToText', () => {
it('identifier (jsx and outermost)', () => {
const identifier = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const actualResult = jsxElementChildToText(identifier, null, null, 'jsx', 'outermost')
expect(actualResult).toEqual('{anIdentifier}')
})
it('identifier (javascript and outermost)', () => {
const identifier = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const actualResult = jsxElementChildToText(identifier, null, null, 'javascript', 'outermost')
expect(actualResult).toEqual('anIdentifier')
})
it('identifier (jsx and inner)', () => {
const identifier = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const actualResult = jsxElementChildToText(identifier, null, null, 'jsx', 'inner')
expect(actualResult).toEqual('anIdentifier')
})
it('identifier (javascript and inner)', () => {
const identifier = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const actualResult = jsxElementChildToText(identifier, null, null, 'javascript', 'inner')
expect(actualResult).toEqual('anIdentifier')
})
it('property access (not optionally chained, jsx and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier.someProperty',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(propertyAccess, null, null, 'jsx', 'outermost')
expect(actualResult).toEqual('{anIdentifier.someProperty}')
})
it('property access (not optionally chained, javascript and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier.someProperty',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(
propertyAccess,
null,
null,
'javascript',
'outermost',
)
expect(actualResult).toEqual('anIdentifier.someProperty')
})
it('property access (not optionally chained, jsx and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier.someProperty',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(propertyAccess, null, null, 'jsx', 'inner')
expect(actualResult).toEqual('anIdentifier.someProperty')
})
it('property access (not optionally chained, javascript and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier.someProperty',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(propertyAccess, null, null, 'javascript', 'inner')
expect(actualResult).toEqual('anIdentifier.someProperty')
})
it('property access (optionally chained, jsx and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier?.someProperty',
'optionally-chained',
)
const actualResult = jsxElementChildToText(propertyAccess, null, null, 'jsx', 'outermost')
expect(actualResult).toEqual('{anIdentifier?.someProperty}')
})
it('property access (optionally chained, javascript and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier?.someProperty',
'optionally-chained',
)
const actualResult = jsxElementChildToText(
propertyAccess,
null,
null,
'javascript',
'outermost',
)
expect(actualResult).toEqual('anIdentifier?.someProperty')
})
it('property access (optionally chained, jsx and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier?.someProperty',
'optionally-chained',
)
const actualResult = jsxElementChildToText(propertyAccess, null, null, 'jsx', 'inner')
expect(actualResult).toEqual('anIdentifier?.someProperty')
})
it('property access (optionally chained, javascript and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const propertyAccess = jsPropertyAccess(
onValue,
'someProperty',
'cba',
null,
emptyComments,
'anIdentifier?.someProperty',
'optionally-chained',
)
const actualResult = jsxElementChildToText(propertyAccess, null, null, 'javascript', 'inner')
expect(actualResult).toEqual('anIdentifier?.someProperty')
})
it('element access (not optionally chained, jsx and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier[elementIdentifier]',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'jsx', 'outermost')
expect(actualResult).toEqual('{anIdentifier[elementIdentifier]}')
})
it('element access (not optionally chained, javascript and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier[elementIdentifier]',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'javascript', 'outermost')
expect(actualResult).toEqual('anIdentifier[elementIdentifier]')
})
it('element access (not optionally chained, jsx and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier[elementIdentifier]',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'jsx', 'inner')
expect(actualResult).toEqual('anIdentifier[elementIdentifier]')
})
it('element access (not optionally chained, javascript and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier[elementIdentifier]',
'not-optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'javascript', 'inner')
expect(actualResult).toEqual('anIdentifier[elementIdentifier]')
})
it('element access (optionally chained, jsx and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier?.[elementIdentifier]',
'optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'jsx', 'outermost')
expect(actualResult).toEqual('{anIdentifier?.[elementIdentifier]}')
})
it('element access (optionally chained, javascript and outermost)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier?.[elementIdentifier]',
'optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'javascript', 'outermost')
expect(actualResult).toEqual('anIdentifier?.[elementIdentifier]')
})
it('element access (optionally chained, jsx and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier?.[elementIdentifier]',
'optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'jsx', 'inner')
expect(actualResult).toEqual('anIdentifier?.[elementIdentifier]')
})
it('element access (optionally chained, javascript and inner)', () => {
const onValue = jsIdentifier('anIdentifier', 'abc', null, emptyComments)
const element = jsIdentifier('elementIdentifier', 'xyz', null, emptyComments)
const elementAccess = jsElementAccess(
onValue,
element,
'cba',
null,
emptyComments,
'anIdentifier?.[elementIdentifier]',
'optionally-chained',
)
const actualResult = jsxElementChildToText(elementAccess, null, null, 'javascript', 'inner')
expect(actualResult).toEqual('anIdentifier?.[elementIdentifier]')
})
it('complicated case', () => {
const parsedResult: ParsedTextFile = lintAndParse(
'test.js',
`const TestComponent = (props) => <div>{something()?.[another().property?.deeperProperty]}</div>`,
null,
emptySet(),
'do-not-trim-bounds',
'do-not-apply-steganography',
)
const toExpressionOptic: Optic<ParsedTextFile, JSElementAccess> = fromTypeGuard(isParseSuccess)
.compose(fromField('topLevelElements'))
.compose(traverseArray())
.compose(fromTypeGuard(isUtopiaJSXComponent))
.compose(fromField('rootElement'))
.compose(fromTypeGuard(isJSXElement))
.compose(fromField('children'))
.compose(traverseArray())
.compose(fromTypeGuard(isJSElementAccess))
const jsExpression = toFirst(toExpressionOptic, parsedResult)
if (isLeft(jsExpression)) {
throw new Error(`Unable to obtain expression from ${JSON.stringify(parsedResult, null, 2)}`)
} else {
const actualResult = jsxElementChildToText(
jsExpression.value,
null,
null,
'javascript',
'outermost',
)
expect(actualResult).toEqual(`something()?.[another().property?.deeperProperty]`)
}
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function jsxElementChildToText(
outermost: 'outermost' | 'inner',
): string {
function outermostWrapInBraces(value: string): string {
if (outermost === 'outermost') {
if (outermost === 'outermost' && expressionContext === 'jsx') {
return `{${value}}`
} else {
return value
Expand Down Expand Up @@ -74,22 +74,36 @@ export function jsxElementChildToText(
return ''
case 'JS_IDENTIFIER':
return outermostWrapInBraces(element.name)
case 'JS_ELEMENT_ACCESS':
case 'JS_ELEMENT_ACCESS': {
const optionalChainedText = element.optionallyChained === 'optionally-chained' ? '?.' : ''
return outermostWrapInBraces(
`${jsxElementChildToText(
element.onValue,
null,
null,
'javascript',
'inner',
)}[${jsxElementChildToText(element.element, null, null, 'javascript', 'inner')}]`,
)}${optionalChainedText}[${jsxElementChildToText(
element.element,
null,
null,
'javascript',
'inner',
)}]`,
)
case 'JS_PROPERTY_ACCESS':
}
case 'JS_PROPERTY_ACCESS': {
const optionalChainedText = element.optionallyChained === 'optionally-chained' ? '?.' : '.'
return outermostWrapInBraces(
`${jsxElementChildToText(element.onValue, null, null, 'javascript', 'inner')}.${
element.property
}`,
`${jsxElementChildToText(
element.onValue,
null,
null,
'javascript',
'inner',
)}${optionalChainedText}${element.property}`,
)
}
default:
assertNever(element)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ describe('Spy Wrapper Template Path Tests', () => {
"storyboard/scene-1/app:other-app-root/other-inner-div/other-card-instance:other-button-instance:other-button-root": Object {
"name": "div",
},
"storyboard/scene-1/app:other-app-root/other-inner-div/other-card-instance:other-button-instance:other-button-root/2f6": Object {
"storyboard/scene-1/app:other-app-root/other-inner-div/other-card-instance:other-button-instance:other-button-root/e06": Object {
"name": "not-jsx-element",
},
"storyboard/scene-2": Object {
Expand Down Expand Up @@ -682,7 +682,7 @@ describe('Spy Wrapper Template Path Tests', () => {
"storyboard/scene-1/app:other-app-root/other-inner-div/other-card-instance:other-button-instance:other-button-root": Object {
"name": "div",
},
"storyboard/scene-1/app:other-app-root/other-inner-div/other-card-instance:other-button-instance:other-button-root/2f6": Object {
"storyboard/scene-1/app:other-app-root/other-inner-div/other-card-instance:other-button-instance:other-button-root/e06": Object {
"name": "not-jsx-element",
},
"storyboard/scene-2": Object {
Expand Down
Loading

0 comments on commit d9411e8

Please sign in to comment.