Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Config][FrameworkBundle] Generate JSON schema for config#59620
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
80efba3
tod7ab6dc
CompareGromNaN commentedJan 26, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thank you very much for working on this feature.
Here is the files that are not validated by the generated JSON schema.
|
24fdcc8
to2d3536a
Comparevaltzu commentedJan 26, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for quick review/comment@GromNaN! Resolved The validator issue still on to-do list, not sure on what basis to allow the empty array – The doctrine issue seems to be caused bynormalization in a closure, some possible solutions below 👇
|
Thanks, perfect.
I think the JSON schema can be a bit more strict than the config validation rules. But this error may be hiding something else.
Arbitrary additional properties are not allowed by the Config component. This would fail the purpose of the JSON schema.
It's the last option if you really can't do any better.
I'd prefer to be able to generate the xsd rather than use it. Maybe we don't need to support this for a first version, since there's an alternative syntax (more verbose). |
84f3965
to008ee37
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I did a quick code review.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigSchemaCacheWarmer.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Config/Tests/Definition/JsonSchemaGeneratorTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
3cfc28e
tode09b68
CompareDid a whole lot of cleanup based on the comments. I also managed to workaround the Doctrine issue by allowing any type when This of course means we don't get a schema for those, but I think it's better than false-positives when using example configuration. Also added a deprecation message to the schema which shows up quite nicely now |
Awesome, the deprecation message is exactly why I want such schema validation. |
valtzu commentedFeb 3, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'm a little stuck with the support for env-specific extensions: since the schema is generated inside the kernel, it does not seem possible to generate schemas for other environments. And since the kernel class is not really exposed anywhere, it's not trivial to create multiple instances of the kernel with different envs (not that it would be a great idea anyway). Best I could think of is
This could look something like
The downside with this solution is that unless you build the container for those other envs, you also don't get the schema for those. Also, I doubt that many people build the container locally for any env but If someone has a better approach, great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could look atbundles.php
to find all the environments and bundles used, but that wouldn't cover generic use cases (if bundles are registered directly in Kernel.php, for example).
We could also scan all the directories and find all the classes that implement theBundleInterface
, but that would require scanning composer autoloader and would take up too many bundles.
I think the idea of generating independent schema files for each env is more realist:
- generate
var/cache/dev/config.schema.json
,var/cache/test/config.schema.json
andvar/cache/prod/config.schema.json
. - when a file is generated, create or update the parent
var/cache/config.schema.json
to include the generated fields.
I tried this with PHPStorm, but the file loading doesn't work yet.
{"$schema":"http://json-schema.org/draft-06/schema#","allOf": [ {"$ref":"./dev/config.schema.json" }, {"$ref":"./test/config.schema.json" } ]}
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigSchemaCacheWarmer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
8715b5d
to003b6f9
Comparevaltzu commentedFeb 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yeah, that is sad. I ended up putting everything in a single file, Also switched from So now you'd need to run bin/console --env=devbin/console --env=testbin/console --env=prod etc to populate all the configs – though in reality I think most projects have all the bundles & extensions available in |
Uh oh!
There was an error while loading.Please reload this page.
Generate JSON schema in
%kernel.build_dir%/config.schema.json
for YAML config when the container is built to improve DX.To discuss:
Is there any way to deal with PHP enums (// just allowed a string!php/enum
yaml tag)? At the moment they are just ignored/excluded!php/enum ...
Should we includeprobably no / up to Flex etc# $schema: path/to/schema.json
in generated yaml files?Should we rather bumpalready bumped in[FrameworkBundle] bump symfony/config requirement to 7.3 #59649symfony/config
version constraint insymfony/framework-bundle
than check for class existence? This is kinda optional stuff so I thought we don't need to enforce it.