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 scoped variables for the parameter of an inline component. #4919

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

seanparsons
Copy link
Contributor

@seanparsons seanparsons commented Feb 16, 2024

Problem:
In the case where an inline component is used, for example:

      <FakeAwait data-uid={'fakeawaitelement'}>
        {(something) => {
          console.log('Something callback.')
          return (
            <div
              data-uid={'something-div'}
              style={{
                width: '100%',
                height: '100%',
                background: 'white',
                justifyContent: 'center',
                alignItems: 'center',
              }}
            >
              {something}
            </div>
          )
        }}
      </FakeAwait>

The parameters to that function (in the above case something) aren't made available in the scoped variables for the body of the function.

Fix:
Now we include the same parameter representation that is used in UtopiaJSXComponent in arbitrary blocks and expressions. During the canvas render, those values are additionally included when building the scoped variables by supplying them to the createLookupRender function.

In the process of doing this work, it was discovered that we were losing the destructured parameters from code when performing the Babel transformation. When attempting to parse the destructured parameters in the Babel AST this was the code that the transformer function would have access to:

return arr.map(function (_ref) {
  var n = _ref.n;
  return <View data-uid='aab' thing={n} />
});

This was due to the es2015 Babel preset applying before any of the plugins run (which is where our code lives), which amongst other things includes a plugin that transforms any object destructuring. Even though the ES2015 standard itself includes support for native object destructuring. The fix for this was to switch the preset to es2016 and then fix all the tests as that change resulted in the transpiled JavaScript values to be more representative of the original JavaScript.

Commit Details:

  • The result of propertiesExposedByParams is passed to createLookupRender in most of the call sites so that the parameters are supplied to the elements within arbitrary expressions.
  • Added params field to JSExpressionOtherJavaScript and ArbitraryJSBlock.
  • Added propertiesExposedByParams utility function.
  • Moved parseParam, parseParams and parseBindingName to parser-printer-parsing.ts so they can be called from the logic there as well.
  • parseOtherJavaScript now takes a params parameter to the create parameter.
  • parseOtherJavaScript parses out the params of single functions arbitrary blocks.
  • Changed Babel transform preset from es2015 to es2016.

…ne component.

- The result of `propertiesExposedByParams` is passed to `createLookupRender`
  in most of the call sites so that the parameters are supplied to the elements
  within arbitrary expressions.
- Added `params` field to `JSExpressionOtherJavaScript` and `ArbitraryJSBlock`.
- Added `propertiesExposedByParams` utility function.
- Moved `parseParam`, `parseParams` and `parseBindingName` to `parser-printer-parsing.ts`
  so they can be called from the logic there as well.
- `parseOtherJavaScript` now takes a `params` parameter to the `create` parameter.
- `parseOtherJavaScript` parses out the params of single functions arbitrary blocks.
Copy link
Contributor

github-actions bot commented Feb 16, 2024

Try me

Copy link

relativeci bot commented Feb 16, 2024

Job #10491: Bundle Size — 62.17MiB (~+0.01%).

36d7a38(current) vs d299c8d master#10488(baseline)

Warning

Bundle contains 58 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
Job #10491
     Baseline
Job #10488
Regression  Initial JS 35.21MiB(~+0.01%) 35.21MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 21.94% 20.35%
No change  Chunks 28 28
No change  Assets 32 32
No change  Modules 4360 4360
No change  Duplicate Modules 494 494
No change  Duplicate Code 30.8% 30.8%
No change  Packages 467 467
No change  Duplicate Packages 58 58
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
Job #10491
     Baseline
Job #10488
Regression  JS 62.16MiB (~+0.01%) 62.15MiB
Improvement  HTML 11.37KiB (-0.32%) 11.41KiB

View job #10491 reportView feature/scoped-variables-inline-... branch activityView project dashboard

Copy link
Contributor

github-actions bot commented Feb 16, 2024

Performance test results:
(Chart1)
(Chart2)

@seanparsons seanparsons merged commit 0b4e98d into master Feb 20, 2024
17 checks passed
@seanparsons seanparsons deleted the feature/scoped-variables-inline-component branch February 20, 2024 14:46
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