Skip to content

Refactor twig component syntax #1

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

Conversation

weaverryan
Copy link

Hi!

Thanks again for getting this rolling! The regex was working great, but it didn't give us any opportunity to give the user a good error if they make a small mistake. This is an attempt to rewrite the logic without regex. It's possible I've missed something (though tests pass). And probably a lot more edge case tests could be added & probably we should try to "mess up" Twig syntax a bunch of times to make sure we get good errors (and ideally write tests for those errors).

This also adds the concept of a "default" block:

<t:successAlert type="{{ alertType }}">
    <p>It's a small world after all</p>
    <t:successAlert type="success">Inception!</t:successAlert>

    <t:block name="some_other_block">
        Not used
    </t:block>
</t:successAlert>

In this case, the content that is NOT inside the <t:block (so the <p> tag + the embedded <t:successAlert type="success">Inception!</t:successAlert>) become part of a new block called default:

<div{{ attributes.defaults({class: 'alert alert-'~type}) }}>
    {% block default %}{% endblock %}
</div>

🔥

@WebMamba
Copy link
Owner

Woooow Thanks for your work on this Ryan! 🚀 After your comment on getting better errors on small developer mistakes, I was spinning my head. And I came to the same conclusion as you: regex is not the way to go, and we need to reproduce what the twig Lexer does!

@WebMamba
Copy link
Owner

Sorry, I just push on the previous branch just before your opened this PR.

@WebMamba WebMamba merged commit c658586 into WebMamba:webmamba/new_syntax_for_twig_component Apr 12, 2023
WebMamba pushed a commit that referenced this pull request Aug 31, 2023
I have a custom hydration extension that in `supports` method checks if the provided class has my custom marker attribute through Reflection, but SA complains:

> Parameter #1 $objectOrClass of class ReflectionClass constructor expects class-string<T of object>|T of object, string given.
WebMamba pushed a commit that referenced this pull request Aug 31, 2023
…ExtensionInterface (norkunas)

This PR was merged into the 2.x branch.

Discussion
----------

[LiveComponent] Add missing typehints to HydrationExtensionInterface

| Q             | A
| ------------- | ---
| Bug fix?      | no /not sure
| New feature?  | no /not sure
| Tickets       | N/A
| License       | MIT

I have a custom hydration extension that in `supports` method checks if the provided class has my custom marker attribute through Reflection, but SA complains:

> Parameter `#1` $objectOrClass of class ReflectionClass constructor expects class-string<T of object>|T of object, string given.

Commits
-------

7f8279c [LiveComponent] Add missing typehints to HydrationExtensionInterface
WebMamba pushed a commit that referenced this pull request May 4, 2024
…ceLoader if the parent $loader is null (vesselind)

This PR was merged into the 2.x branch.

Discussion
----------

symfony#1770 [Autocomplete] Ignore the ExtraLazyChoiceLoader if the parent $loader is null

| Q             | A
| ------------- | ---
| Bug fix?      | yes/no
| New feature?  | no
| Issues        | Fix symfony#1770
| License       | MIT

When a custom autoload field class is created and `choices` option is passed, an error occurs: `Symfony\UX\Autocomplete\Form\ChoiceList\Loader\ExtraLazyChoiceLoader::__construct(): Argument #1 ($decorated) must be of type Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface, null given`.

This should fix the problem, described in symfony#1770.

Commits
-------

28af7fc symfony#1770 ignore the ExtraLazyChoiceLoader if the parent $loader is null
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.

2 participants