Skip to content

[TwigComponent] Add tests ComponentParser should pass #1236

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

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

smnandre
Copy link
Member

As the name resolution must be delayed from "parse-time" to "render-time", those tests should then pass.

As we know, they currently fail for

  • HTML full tags: <twig:XYZ></twig:XYZ>
  • Twig tag: {% component XYZ %}

weaverryan added a commit that referenced this pull request Nov 6, 2023
…nent rendered vs {{ component() (weaverryan)

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

Discussion
----------

[TwigComponents] Fixing inconsistency with how {% component rendered vs {{ component()

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Issues        | Fix #1205 (?) and maybe others?
| License       | MIT

tl;dr `{% component` was rendered slightly differently than `{{ component`. Additionally, for `{% component`, the template was determined too early, which didn't allow for the template to be overridden via an event.

**Longer Explanation**

This cleans up `ComponentTokenParser`, removing some unnecessary parts. We now always set the "embedded" component parent to a magic `__parent__` variable. This allows us to delay the determination of the component template until runtime, after the `PreRenderEvent` has been called.

TODO:

* [ ] Bring in tests from #1236 and #1237 and check other tests
* [ ] Check #1205 to verify this fixes
* [ ] Check debug:twig-component command for any changes needed

Cheers!

Commits
-------

df6388e [TwigComponents] Fixing inconsistency with how {% component rendered vs {{ component()
@smnandre smnandre force-pushed the dx/add-component-parser-tests branch from ac8474b to 8a97b7e Compare November 6, 2023 21:05
@smnandre
Copy link
Member Author

smnandre commented Nov 6, 2023

Ready here @weaverryan ... and the tests are green thanks to you :)

@weaverryan weaverryan force-pushed the dx/add-component-parser-tests branch from 8a97b7e to 05f9ef9 Compare November 7, 2023 18:46
@weaverryan
Copy link
Member

Thanks Simon!

@weaverryan weaverryan merged commit 9cbaeee into symfony:2.x Nov 7, 2023
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