Refactoring development/front-end-build time code away from vaadin-server #22748
Conversation
And from vaadin-spring where it leaked into via dev-server.
No more used in actual artifacts
…ow-server Most of this should be in vaadin-dev 🤔
…rom flow-server Claude initiated, not finished yet, not even the tests compile...
Now basic usage seems to work again after refactoring.
(after rebase)
|
Test Results1 172 files - 119 1 172 suites - 119 1h 9m 44s ⏱️ - 6m 28s For more details on these failures and errors, see this check. Results for commit 4dc58c4. ± Comparison against base commit ada6b1a. This pull request removes 1171 tests. |
| private boolean dependencyHasTagName(Dependency dependency, String tag) { | ||
| String url = FilenameUtils.removeExtension(dependency.getUrl()) | ||
| .toLowerCase(Locale.ENGLISH); | ||
| String url = dependency.getUrl(); |
There was a problem hiding this comment.
We should not do it like this. If we want to have our own utils instead of using one util method from a dependency, then let's add a util method and let's not add the implementation inline in various places
| <scope>provided</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.vaadin</groupId> |
There was a problem hiding this comment.
This sounds wrong. If the frontend tools module exists, it should only be used from the dev server and from the Maven/Gradle plugins
There was a problem hiding this comment.
What is flow-polymer2lit module really doing 🤔 Probably pulled in elsewhere as well. Need to check.
| try { | ||
| FileUtils.write(featureFlagFile, properties.toString(), | ||
| StandardCharsets.UTF_8); | ||
| java.nio.file.Files.writeString(featureFlagFile.toPath(), |
There was a problem hiding this comment.
To make things easier, this and similar fixes should be made as a separate PR. That PR is trivial to review and merge - contrary to this one
There was a problem hiding this comment.
This is a draft PR to explore the viability, as discussed elsewhere and started from totally different angle. In hindsight everybody would do it in batches (order really doesn't matter) 🧸
mshabarov
added
the
Contribution
…dencies (#22758) commons-compress dependency is only used to download front-end build tooling. Due to non-optimal modularisation this code is in flow-server and it leaks into production deployments (see #22778). This change does not properly fix that large issue, but can be considered as a start to work towards fixing it. Took this alternative approach as cleanin up the whole development/compilation time code from vaadin-server appears to be too large piece to tackle at once (see to gigantic #22748 ). This only moves parts that use commons-compress to a separate module (provided scope), users must bring it to classpath (plugins/dev-mode). Removing commons-io from flow-server ought to be rather straightforward after this change, but a lot of code would still be in wrong place and e.g. #22238 would still be on the tasklist. But, these are all marked "internal classes", so we should be able to do cleanup even during 25 without event changing dependencies.
Description
Moves stuff that really never should have been in vaadin-server module to a separate module and tries to keep minimal amount of that code in vaadin-server.
Also removes commons-io and commons-compress dependencies from vaadin-server (and this way from production mode Vaadin apps) as those are now either not needed or can be replaced with trivial code from newer JDKs.
Current state: early prototype, tested in local projects to work at least to some extent.
Type of change
Checklist
Additional for
Featuretype of change