Refactoring development/front-end-build time code away from vaadin-server#22748
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.
|
Done in #23161 |
3 participants
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