Skip to content

Conversation

ottaviano
Copy link

@ottaviano ottaviano commented Nov 2, 2024

Q A
Bug fix? no
New feature? yes
Issues ~
License MIT

With the nested attributes feature, components have become incompatible with the AlpineJS library, which also uses HTML attributes in a similar format, such as x-bind:class="foo" or x-transition:enter="ease-out duration-300".
This PR introduces a way to specify that an attribute containing : and ending with ! should not be treated as a nested attribute.

Propose :

<twig:Button x-bind:class!="hey">My button</twig:Button>

Copy link
Contributor

@WebMamba WebMamba left a comment

Choose a reason for hiding this comment

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

Could you add some documentation ? What do you think about a section on how to use TwigComponent with Alpine js ?

@smnandre
Copy link
Member

smnandre commented Nov 2, 2024

Hi @ottaviano, thank you for this PR!

We have special handling for aria attributes, classes, stimulus attributes, etc.

I see not problem if we add a special case if a given attribute key:

  • start with x-
  • contains a :

(and I still consider that "x-bind" cannot be a component name)

So i'm still thinking we should have a special treatment here.. because even if the solution you suggest is smart, i really don't like to open the regex like this and allow new caracters here. :|

What do you think ?

@Kocal
Copy link
Member

Kocal commented Nov 2, 2024

Do we agree this issue would not occurs if the "nested attributes" feature didn't exist? :/

@smnandre
Copy link
Member

smnandre commented Nov 2, 2024

Once we release and document a feature, we can’t just remove it without consideration—even if I’d love to, especially to resolve some Twig/Live issues! 🙂

@ottaviano
Copy link
Author

managing each library (like AlpineJs, Vue, etc) independently can add complexity to the code, as we may need to revisit and modify it if a new conflict arises with another library in the future. My approach is to simply enable/disable a nested attribute, ensuring compatibility with any JavaScript library.

@Kocal
Copy link
Member

Kocal commented Nov 3, 2024

Yes, but it add (again) a new syntax for attributes, that should be documented and known by users.

With #2328, yes it add more work and complexity for us (UX mainteners), but users will be able to copy/paste examples valid Alpine (and others) code without trying to understand why the code does not work when they use the special attributes on a Twig component:
image

@ottaviano
Copy link
Author

I see your point; I close this PR in favor of #2328
Thank you for your work, can’t wait to test the improvement !

@ottaviano ottaviano closed this Nov 3, 2024
@ottaviano ottaviano deleted the disable-dynamic-props branch November 3, 2024 12:27
@smnandre
Copy link
Member

smnandre commented Nov 3, 2024

Yep, nothing to do with your PR, i was thinking of DX and incoming support .. i should have been more explicit sorry @ottaviano :)

Kocal added a commit that referenced this pull request Nov 23, 2024
… (smnandre)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[TwigComponent] Ignore "nested" for Alpine & Vue attributes

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #1839
| License       | MIT

Have lost time on Twig & the website 😓

Alternative implementation of #2325

Update: Improved `__toString` performance following `@Kocal` comments

---
Now all these attributes are directly rendered

| Framework       | Prefix | Code Example                                                                                       | Documentation                                                 |
|-----------------|---------|-------------------------------------------------------------------------------------------------------|----------------------------------------------------------------|
| **Alpine.js**   | `x-`    | `<div x-data="{ open: false }" x-show="open"></div>`                                                 | [Documentation Alpine.js](https://alpinejs.dev/)               |
| **Vue.js**      | `v-`    | `<input v-model="message" v-if="show">`                                                              | [Documentation Vue.js](https://vuejs.org/guide/)               |
| **Stencil**     | `@`     | `<my-component `@onClick`="handleClick"></my-component>`                                               | [Documentation Stencil](https://stenciljs.com/docs/)           |
| **Lit**         | `@`     | `<button `@click`="${this.handleClick}">Click me</button>`                                             | [Documentation Lit](https://lit.dev/docs/)                    |

Commits
-------

7bd2854 Improve __toString performance
cb7809f Add str_contains in nested method
9f7c9c8 Performance
3452436 fabbot
1ed1db7 [TwigComponent] Ignore "nested" for Alpine & Vue attributes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New Feature Status: Needs Review Needs to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants