Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support parsing components with top level if statements #4696

Merged
merged 6 commits into from
Jan 10, 2024

Conversation

Rheeseyb
Copy link
Contributor

@Rheeseyb Rheeseyb commented Jan 5, 2024

Problem:
Our parsing currently fails on a component with a top level if statement, meaning that a component of the form:

const Card = () => {
  if (true) {
    return <div>true</div>
  } else {
    return <div>false</div>
  }
}

will be parsed as an arbitrary JS block, meaning that it then can't be focused, and therefore internal elements can't be selected.

Fix:
Since this is a general issue, where the component will fail to parse if there is some arbitrary javascript code surrounding the return statement, we now support parsing a component with a JSExpressionOtherJavaScript as the root element. To do this, when parsing a function body, if we first fail to parse out any top level JSX elements (in the return statement), we then attempt to parse the entire body itself as JSExpressionOtherJavaScript, and check if there are elements defined within - if there are none then we fall back to treating the entire function as a top level JS block.

When we are parsing an isolated piece of code somewhere in the file, part of the existing logic includes a babel transpile that might also wrap it in parens (as perhaps that piece of code in isolation would be incorrect, but in the wider context of the full source file it is perfectly valid). We have a similar issue here, as we're parsing the entire function body, and e.g. a return statement would not be legal at the root level of a source file. For this reason, we can now also wrap the isolated code into an anonymous function purely for the sake of transpiling and parsing it.

I've also had to tweak a small amount of the printing logic to handle printing of components with a JSExpressionOtherJavaScript.

Caveat:
As the root element is now treated as being generated by code, and we don't allow canvas manipulation of generated elements, these components will currently only support focusing / selection, but not editing.

Sample project:
Before: https://utopia.pizza/p/8830bc6c-separate-marjoram
After: https://utopia.pizza/p/8830bc6c-separate-marjoram/?branch_name=feature-parse-components-with-top-level-if

Copy link
Contributor

github-actions bot commented Jan 5, 2024

Try me

Copy link

relativeci bot commented Jan 5, 2024

Job #9784: Bundle Size — 62.2MiB (~+0.01%).

aebfa59(current) vs df0030c master#9783(baseline)

Warning

Bundle contains 66 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
Job #9784
     Baseline
Job #9783
Regression  Initial JS 45.4MiB(~+0.01%) 45.4MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 21.77% 0%
No change  Chunks 26 26
No change  Assets 30 30
No change  Modules 4362 4362
No change  Duplicate Modules 471 471
No change  Duplicate Code 30.75% 30.75%
No change  Packages 462 462
No change  Duplicate Packages 65 65
Bundle size by type  Change 1 change Regression 1 regression
                 Current
Job #9784
     Baseline
Job #9783
Regression  JS 62.19MiB (~+0.01%) 62.18MiB
Not changed  HTML 11.54KiB 11.54KiB

View job #9784 reportView feature/parse-components-with-to... branch activity

Copy link
Contributor

github-actions bot commented Jan 5, 2024

Performance test results:
(Chart1)
(Chart2)

@Rheeseyb Rheeseyb changed the title (Draft) Support parsing components with top level if statements Support parsing components with top level if statements Jan 9, 2024
@Rheeseyb Rheeseyb changed the title Support parsing components with top level if statements (Draft) Support parsing components with top level if statements Jan 9, 2024
@Rheeseyb Rheeseyb changed the title (Draft) Support parsing components with top level if statements Support parsing components with top level if statements Jan 9, 2024
@Rheeseyb Rheeseyb marked this pull request as ready for review January 9, 2024 16:12
@Rheeseyb Rheeseyb merged commit 625692b into master Jan 10, 2024
14 checks passed
@Rheeseyb Rheeseyb deleted the feature/parse-components-with-top-level-if branch January 10, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants