fix: Also take into account the application.properties value #19966
Conversation
| // If we are enabling react we should for spring projects check the | ||
| // application.properties file flag also. | ||
| File resource = new File(buildDirectory, | ||
| "classes/application.properties"); |
There was a problem hiding this comment.
application.yml/yaml are not supported?
There was a problem hiding this comment.
This is more of a test at the moment and as flow doesn't have any yaml handling libraries it would be a build by hand parsing.
Also not certain if the linking to spring specific property files is a good idea as it might lead to expectations in overall configuration.
There was a problem hiding this comment.
This would also leave out all profile specific properties files application-{profile}.properties like application-prod.properties, application-dev.properties etc.
There was a problem hiding this comment.
yeah that's also true.. I'm wondering if there could be some kind of way to get the available spring environment in the build so that spring would handle all their "property magic"
There was a problem hiding this comment.
The main problem with possibly trying to use spring tools is that the data is needed during plugin build time also and not just runtime.
There was a problem hiding this comment.
One possibility could be to add to starter projects into pom.xml <properties><reactEnable>true</reactEnable></properties> and have in the default application.properties vaadin.react.enable=@reactEnable@.
This would sync the pom.xml and application.properties values for the flag.
The problem here is ofc that it could be removed by mistake and wouldn't help old projects.
There was a problem hiding this comment.
Doesn't this require additional configuration? Like filtering of resources to work?
There was a problem hiding this comment.
Actually it works out of the box for spring-boot: https://docs.spring.io/spring-boot/docs/2.0.0.M7/reference/html/howto-properties-and-configuration.html#howto-automatic-expansion-maven
If you use the spring-boot-starter-parent, you can then refer to your Maven ‘project properties’ with @..@ placeholders
There was a problem hiding this comment.
Without the starter parent you would need resource filtering and plugin delimiter configuration.
There was a problem hiding this comment.
vaadin.react.enable=@reactEnable@ looks good idea, but polluting starters with a property that most don't need is not so nice.
If the point is to avoid issues with vaadin.react.enable being added only to application.properties then better documentation could be enough. Also this @reactEnable@ should be mentioned somewhere.
|
When checking for react enable
also look if we have a application.properties
with the vaadin.react.enable flag.
This is especially for production build when only
application.properties has react disabled.
Touches #19951 and #19911