I think this library mostly aims API first or API only projects. Twig is an unnecessary dependency with enable_swagger_ui: false but still needed for api_platform.swagger.action.ui service.
I believe we can drop Twig entirely even when using the Swagger UI by ensuring that it's fully client-side.
In fact I think separating "Swagger UI for Symfony" project can be more reasonable.
Yes, I see no reason for this project to include any JS and CSS, etc.
It should live in a separate frontend project, like https://github.com/api-platform/admin and https://github.com/api-platform/generate-crud. It should be included as part of our Docker Compose setup. And with Symfony Flex it should be pretty easy to advise the user to install it separately (or even auto-install?).
@api-platform/core-team One guiding principle for web APIs is the separation of the frontend and the backend. I believe we need to set a good example ourselves.
I definitely agree with that, but it's easiest solution right now. We can work to do it in 2.2!
What if we use EasyAdminBundle or another bundle which requires Twig ?
Personally, I don't use api-platform-admin but EasyAdminBundle because my frontend project is in typescript with react. So I can't add easily api-platform-admin. And I don't want to have two projects : one for the app in typescript and another one for the admin with js.
@pierre-H this doesn't mean that you can't have twig as a dependency in your project :). This is just a proposal to drop the ~hard~ soft dependency on twig on the core library.
There is no hard dependency on Twig in core. It's just a soft dep.
Actually the Symfony bridge's service "api_platform.swagger.action.ui" introduce a hard dependency with Twig (still true is the 2.2-beta).
The "cleanest" fix i could think of, would be to remove that service using a compiler pass when the twig service definition doesnt exists ?
@fgrandjean Do you have some time to implement this ?
@Simperfit I can, but i do need some direction : Do we go for a clearer exception that tell end user that when used with Symfony they are required to add twig as a deps or do we silently toggle on/off the Swagger ui based on deps (as the Sf FrameworkBundle does) ?
@fgrandjean I think we need to toggle silently off when twig is not part of the project.
WDYT @api-platform/core-team ?
api_platform.swagger.action.ui should be moved to swagger-ui.xml. This way if enable_swagger_ui is false, it'd not be loaded.
Taking a closer look it seems swagger-ui is not the only hard dependency on twig. "api_platform.graphql.action.entrypoint" also introduce a hard dependency.
So, i guess that if TwigBundle is not available we should toggle "enable_swagger_ui" and "graphql" to false in config ?
It's the same for graphiql it's ui should be loaded separatly
We should replace Twig by the Symfony Templating component and its PHP templating engine! 😋
Twig dependency is dev-only now so I guess this can be closed.
looks like it's not closed... :sob:
Symfony version: 5.1.3
api-platform/core v2.5.6
Here's what I get
Script cache:clear returned with error code 1
!!
!! // Clearing the cache for the prod environment with debug
!! // false
!!
!!
!! In CheckExceptionOnInvalidReferenceBehaviorPass.php line 86:
!!
!! The service "api_platform.swagger.action.ui" has a dependency on a non-exis
!! tent service "twig".
!!
!!
!! cache:clear [--no-warmup] [--no-optional-warmers] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--] <command>
!!
!!
Script @auto-scripts was called via post-install-cmd
I can help making the PR next week if somebody can guide me a bit about what to do.
I guess that about swagger ui action we can somehow conditionnaly load the swagger-ui.xml file.
But then... if I comment it to test I'm facing the same issue but this time with graphiql.
Executing script cache:clear [KO]
[KO]
Script cache:clear returned with error code 1
!!
!! In CheckExceptionOnInvalidReferenceBehaviorPass.php line 86:
!!
!! The service "api_platform.graphql.action.graphiql" has a dependency on a no
!! n-existent service "twig".
!!
!!
!!
Script @auto-scripts was called via post-install-cmd
I've not find at the moment where I could fix this one, can anybody help me?
I finally wiped out the errors but this is a lot of things to conditionnally load depending if twig is installed or not. Especially at the moment looks like it's not easy when graphql is enabled to not install twig as a production requirement.
So... should I throw exceptions when twig is not available but graphql enabled or swagger ui enabled? :thinking:
Ok, so I've found this PR that has never been merged and that looks like a good starting point: https://github.com/api-platform/core/pull/1829
@dunglas @teohhanhui I've found a fix by overriding swagger ui action service in my own application:
services:
# this service is not lazy and requires twig so let's override it
api_platform.swagger.action.ui:
class: ApiPlatform\Core\Bridge\Symfony\Bundle\Action\SwaggerUiAction
So... if you want I can make a PR but I see at least 2 to do things:
api_platform.swagger.action.ui lazy.class_exists() calls about twig and throw exceptions to inform during compiler passes to install twig when you enable some service (swagger ui, graphql) that requires it.Which one do you prefer? (Alternatives accepted).
Most helpful comment
@dunglas @teohhanhui I've found a fix by overriding swagger ui action service in my own application:
So... if you want I can make a PR but I see at least 2 to do things:
api_platform.swagger.action.uilazy.class_exists()calls about twig and throw exceptions to inform during compiler passes to install twig when you enable some service (swagger ui, graphql) that requires it.Which one do you prefer? (Alternatives accepted).