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

feat(template): support spread syntax ... #1965

Merged
merged 11 commits into from
May 11, 2024
Merged

feat(template): support spread syntax ... #1965

merged 11 commits into from
May 11, 2024

Conversation

bigopon
Copy link
Member

@bigopon bigopon commented May 8, 2024

📖 Description

When the number of bindable properties grows bigger, and the binding are repetitive, it's quite a boilerplate having to specify everything, as per the examples in #1477 #1258 :

Desirable:

  <k-button  repeat.for="action of actions" ...action>${action.content}</k-button>
  <k-button  repeat.for="action of actions" ...$bindables="action">${action.content}</k-button>
  <k-button  repeat.for="action of actions" $bindables.spread="action">${action.content}</k-button>

Not desirable

  <k-button color.bind="action.color"
            size.bind="action.size"
            type.bind="action.type" 
            repeat.for="action of actions">${action.content}</k-button>  

This PR adds support for spread syntaxes:

  • full: $bindables.spread="expression"
  • short: ...$bindables="expression"
  • shorter: ...expresion

Note:

  • binding created via $bindables will have mode to-view, ignoring any default mode of the target bindable properties
  • If the same object is returned from evaluating the expression, the spread binding won't try to rebind its inner bindings. This means mutating and then reassigning won't result in new binding, instead, give the spread binding a new object.
  • fix an issue with spreading ...$attrs not consider element bindable properties before custom attribute

🎫 Issues

Close #1477
Close #1258

cc @fkleuver @Sayan751 @brandonseydel @Vheissu

Copy link

codecov bot commented May 8, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 88.57%. Comparing base (eaf2cd7) to head (16a6bb3).
Report is 4 commits behind head on master.

Files Patch % Lines
...ackages/runtime-html/src/binding/spread-binding.ts 88.05% 8 Missing ⚠️
...-html/src/resources/template-controllers/portal.ts 0.00% 1 Missing ⚠️
...ackages/template-compiler/src/template-compiler.ts 97.91% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1965      +/-   ##
==========================================
- Coverage   88.62%   88.57%   -0.05%     
==========================================
  Files         272      272              
  Lines       22823    22909      +86     
  Branches     5292     5302      +10     
==========================================
+ Hits        20226    20292      +66     
- Misses       2597     2617      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bigopon
Copy link
Member Author

bigopon commented May 8, 2024

@bigopon bigopon changed the title Feat/spread feat(template): support spread syntax ... May 8, 2024
@brandonseydel
Copy link
Member

brandonseydel commented May 8, 2024

How does order work or is there an error? e.g.

<mb-button small.bind="false" $bindables.spread="{small:true}" />

@bigopon
Copy link
Member Author

bigopon commented May 8, 2024

How does order work or is there an error? e.g.

<mb-button small.bind="false" $bindables.spread="{small:true}" />

It will create 2 binding in the order seen in the template.

Copy link
Member

@Vheissu Vheissu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @bigopon it's actually awesome how very little code this is to implement.

@Sayan751
Copy link
Contributor

Sayan751 commented May 9, 2024

Thanks @bigopon for this nice addition!

@bigopon
Copy link
Member Author

bigopon commented May 9, 2024

@fkleuver @Sayan751 I ended up giving a special treatment to ... syntax, embed the knowledge of it into the compiler. I think it's more appropriate this way, as there are syntax, parser, command, instruction and order matters to juggle, separating the work out into different concepts likely gonna results in leaky abstraction.

@bigopon bigopon merged commit ccae63b into master May 11, 2024
29 checks passed
@bigopon bigopon deleted the feat/spread branch May 11, 2024 00:32
@bigopon
Copy link
Member Author

bigopon commented May 11, 2024

@Sayan751 I'll go ahead with a new beta for the weekend, but will address any comments you have.

AureliaEffect pushed a commit that referenced this pull request May 11, 2024
2.0.0-beta.17 (2024-05-11)

**BREAKING CHANGES:**

* **template:** auto infer binding expression when empty (#1963) ([3359939](3359939))

    Previously only the expression of binding to element bindables get auto inferred, now it's expanded to all bindings
    with `.bind`/`.to-view`/`.from-view`/`.two-way`/`.one-time`
    Examples:
    ```html
    <div some-prop.bind=""> means <div some-prop.bind="someProp">
    <div some-prop.bind> means <div some-prop.bind="someProp">
    <div some-prop.one-time> means <div some-prop.one-time="someProp">
    ...
    ```
* **convention:** rewrite runtime-html decorators (#1960) ([eaf2cd7](eaf2cd7))

    With tooling in the instable state for the tc39 decorator support, we will generate standard fn call code instead of decorator.
    This will likely be changed when browsers start officially supporting it, or at least when the tooling (both spec & tooling stability + compat) gets better

**Features:**

* **template:** support spread syntax with `spread` command and ... (#1965) ([ccae63b](ccae63b))
* **repeat:** allow custom repeatable value (#1962) ([c47df91](c47df91))

**Bug Fixes:**

* **compiler:** fix order when spreading custom attribute into element bindable ([ccae63b](ccae63b))
* **au-slot:** separate parent scope selection from host scope selection (#1961) ([ff605fb](ff605fb))

**Refactorings:**

* **kernel:** mark side effect free (#1964) ([22c8f71](22c8f71))
Copy link
Contributor

@Sayan751 Sayan751 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @bigopon! Just some minor comments and questions.

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.

Ability to spread an object to bindables [RFC] Bindables built in attribute (Spread like)
4 participants