Skip to content

[WebProfilerBundle] Mark CodeExtension as non-internal #52531

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 18, 2023

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 7.0
Bug fix? no
New feature? no
Deprecations? no
Issues -
License MIT

From #52449 (comment) by @GromNaN

Using those twig helpers in a profiler page is 100% legit. I think that the notice in the class' docbloc + the fact it moved to the WebProfilerBundle namespace allows us to mark the class back as non-internal.

@GromNaN
Copy link
Member

GromNaN commented Nov 10, 2023

It's also used in symfony/demo. https://github.com/symfony/demo/blob/206f0c9aab07cdc9f7437640b12947d35e9d5e72/templates/debug/source_code.html.twig#L33-L34
It will break if the demo is run with env=prod.

@nicolas-grekas
Copy link
Member Author

Demo needs to adjust, we don't want to support such use cases.

@wouterj
Copy link
Member

wouterj commented Nov 10, 2023

If we don't want users to use this functions/filters, should we remove them from the twig reference?

@GromNaN
Copy link
Member

GromNaN commented Nov 10, 2023

If we don't want users to use this functions/filters, should we remove them from the twig reference?

Or add a warning that those are only available for web profiler.

Is it a possible and good idea to register the extension conditionally in the profiler controller?

@stof
Copy link
Member

stof commented Nov 10, 2023

We cannot register extensions conditionally. Twig does allow registering new extensions after initialization is done, so this would be very fragile (and would break if your kernel handles multiple requests without a full shutdown for instance). It would also break the twig linting as it would complain about those functions not being available.

For the documentation, it would make sense to document that those functions are defined by WebProfilerBundle and not by TwigBundle. But I would not remove them entirely. Bundles providing custom profiler panels can legitimately use them.

@fabpot
Copy link
Member

fabpot commented Nov 18, 2023

Thank you @nicolas-grekas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants