Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New flow-only project without hilla and react-components application test module #5060

Open
knoobie opened this issue Feb 16, 2024 · 15 comments

Comments

@knoobie
Copy link
Contributor

knoobie commented Feb 16, 2024

Describe your motivation

Add a new test module into the platform and smoke tests for flow-only application that exclude hilla and react components.

To verify that this really works; the new SBOM Goal of flow could be executed and the created SBOM inspected that hilla and react-components are not there.

Example Pom:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>com.example</groupId>
  <artifactId>flow-only-skeleton</artifactId>
  <name>Project base for Spring Boot and Vaadin Flow without Hilla and React Components</name>
  <version>1.0-SNAPSHOT</version>
  <packaging>jar</packaging>

  <properties>
    <java.version>17</java.version>
    <vaadin.version>24.4+++</vaadin.version>
  </properties>

  <parent>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-parent</artifactId>
    <version>3.2.2</version>
  </parent>

  <repositories>
    <repository>
      <id>Vaadin Directory</id>
      <url>https://maven.vaadin.com/vaadin-addons</url>
      <snapshots>
        <enabled>false</enabled>
      </snapshots>
    </repository>
  </repositories>

  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>com.vaadin</groupId>
        <artifactId>vaadin-bom</artifactId>
        <version>${vaadin.version}</version>
        <type>pom</type>
        <scope>import</scope>
      </dependency>
    </dependencies>
  </dependencyManagement>

  <dependencies>
    <dependency>
      <groupId>com.vaadin</groupId>
      <artifactId>vaadin-core</artifactId>
      <exclusions>
        <exclusion>
          <groupId>com.vaadin</groupId>
          <artifactId>vaadin-dev-bundle</artifactId>
        </exclusion>
        <exclusion>
          <groupId>com.vaadin</groupId>
          <artifactId>flow-react</artifactId>
        </exclusion>
      </exclusions>
    </dependency>
    <dependency>
      <groupId>com.vaadin</groupId>
      <artifactId>vaadin-spring-boot-starter</artifactId>
      <exclusions>
        <exclusion>
          <groupId>com.vaadin</groupId>
          <artifactId>vaadin-core</artifactId>
        </exclusion>
        <exclusion>
          <!-- old module -->
          <groupId>com.vaadin.hilla</groupId>
          <artifactId>hilla-react</artifactId>
        </exclusion>
        <exclusion>
          <groupId>com.vaadin</groupId>
          <artifactId>hilla-react</artifactId>
        </exclusion>
      </exclusions>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-validation</artifactId>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-devtools</artifactId>
      <optional>true</optional>
    </dependency>
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-test</artifactId>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>com.vaadin</groupId>
      <artifactId>vaadin-testbench-junit5</artifactId>
      <scope>test</scope>
    </dependency>
  </dependencies>

  <build>
    <defaultGoal>spring-boot:run</defaultGoal>
    <plugins>
      <plugin>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-maven-plugin</artifactId>
      </plugin>

      <plugin>
        <groupId>com.vaadin</groupId>
        <artifactId>vaadin-maven-plugin</artifactId>
        <version>${vaadin.version}</version>
        <executions>
          <execution>
            <goals>
              <goal>prepare-frontend</goal>
            </goals>
          </execution>
        </executions>
        <configuration>
          <!-- Default: true / use React Router instead of Vaadin Router -->
          <reactEnable>true</reactEnable>
          <!-- Default: false / ensure smooth frontend development -->
          <frontendHotdeploy>true</frontendHotdeploy>
        </configuration>
      </plugin>
    </plugins>
  </build>

  <profiles>
    <profile>
      <!-- Production mode is activated using -Pproduction -->
      <id>production</id>
      <dependencies>
        <!-- Exclude development dependencies and react from production -->
        <dependency>
          <groupId>com.vaadin</groupId>
          <artifactId>vaadin-core</artifactId>
          <exclusions>
            <exclusion>
              <groupId>com.vaadin</groupId>
              <artifactId>vaadin-dev-server</artifactId>
            </exclusion>
            <exclusion>
              <groupId>com.vaadin</groupId>
              <artifactId>vaadin-dev-bundle</artifactId>
            </exclusion>
            <exclusion>
              <groupId>com.vaadin</groupId>
              <artifactId>vaadin-dev</artifactId>
            </exclusion>
            <exclusion>
              <groupId>com.vaadin</groupId>
              <artifactId>flow-react</artifactId>
            </exclusion>
          </exclusions>
        </dependency>
      </dependencies>
      <build>
        <plugins>
          <plugin>
            <groupId>com.vaadin</groupId>
            <artifactId>vaadin-maven-plugin</artifactId>
            <version>${vaadin.version}</version>
            <executions>
              <execution>
                <goals>
                  <goal>build-frontend</goal>
                </goals>
                <phase>compile</phase>
              </execution>
            </executions>
            <configuration>
              <!-- Default: true / use React Router instead of Vaadin Router -->
              <reactEnable>true</reactEnable>
              <!-- Default: false / ensure that a production bundle is always created -->
              <forceProductionBuild>true</forceProductionBuild>
            </configuration>
          </plugin>
        </plugins>
      </build>
    </profile>
  </profiles>
</project>

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

Vaadin 24.4

@mstahv
Copy link
Member

mstahv commented Mar 5, 2024

I'm bit worried if this is what typical production Vaadin app builds start to look like 😬

I was also looking into a trimmed down projects with 24.4, as both the amount of dependencies and the artifact size grows significantly with it.

Did you investigate if excluding the react stuff makes a difference for the artifact size or does it only add extra client side dependencies to look after?

Do you happen to know what the reactEnable flag does? No docs for it like there if for most other configuration flags. To me it kind of looks like you'd like that to be false, but at least my project (with alpha8) didn't compile if false was given to it. Complaining something like:

[ERROR] frontend/routes.tsx(1,50): error TS2307: Cannot find module 'react-router-dom' or its corresponding type declarations.
[ERROR] frontend/routes.tsx(2,34): error TS2307: Cannot find module 'Frontend/generated/flow/Flow' or its corresponding type declarations.

@knoobie
Copy link
Contributor Author

knoobie commented Mar 5, 2024

Did you investigate if excluding the react stuff makes a difference for the artifact size or does it only add extra client side dependencies to look after?

I haven't done any real world tests (wanted to checkout the first beta once I have time) - currently all my things are based on knowledge acquired from looking at the PRs in flow and the other repos :)

My educational guess would be:

  • the new default: reactEnable = true
    • Hilla is on the class path -> big increase in app size
    • React Router is used in favor of the Vaadin Router / Lit
    • React Components are added to the package.json -> big dependency graph; problem for SBOM
  • reactEnable = false
    • Hilla stays on the class path -> big increase in app size
    • Also excluding Hilla -> there should no increase expect some kb
    • React Components are not added to the package.json
    • "old" Vaadin Router is used instead of React Router
  • reactEnable = true & excluding flow-react from pom (my way going forward)
    • Hilla stays on the class path -> big increase in app size
    • Also excluding Hilla -> there should no increase at all
    • React Components are not added to the package.json
    • React Router is used over Vaadin Router (cause it looks legacy / not really maintained going in the future)

@mstahv
Copy link
Member

mstahv commented Mar 7, 2024

I created a branch that only exlcudes hilla:
vaadin/skeleton-starter-flow-spring@v24.4...v24.4-flow-only

Not very pretty excludes, but this considerably trims down the jar size. I didn't go as far as you did excluding all react stuff, as then one would again need to get back to npm builds 😬 But this could maybe be basis for reactles project base as well.

The (non-optimised) default front-end bundle grows a bit (1 extra request, ~ 200kb (~ 50kb compressed)). I didn't investigate if that is because of react stuff, but probably. But I'm (personally) ready to take that if I can avoid front-end build.

Looking at excludes in above and e.g. in v24.4 quarkus branch, I would claim that this "unified platform" (whether you love the idea or not) is implemented in a wrong way. These exclusions underline that there is intermediate modules missing for hilla & flow, that would then be pulled together in an other. Now you need an axe to get something you want. There are very good reasons (performance, build time, artifact/fron-end weight, security) to trim down your projects/builds to minimum, and now that gets more complicated than it'd need to.

Also, if existing users don't figure out to add these exclusions, they are getting a ton of new stuff both to their classpath and front-end bundle. I'm bit afraid this way there will be "surprises" when people just upgrade their minor versions. I'd instead of re-using the old module names, create new one. There could be for example be vaadin-platform for the new starter projects, that would pull in everything. This way this release could be considered a minor release, at least for Flow users.

@knoobie
Copy link
Contributor Author

knoobie commented Mar 7, 2024

There are very good reasons (performance, build time, artifact/fron-end weight, security) to trim down your projects/builds to minimum, and now that gets more complicated than it'd need to.

when people just upgrade their minor versions.

Those are two of my main concerns with the current approach. Coming from a highly restricted environment, I have to check and get approvements for everything transitive in my app. Therefore I'm trying to skim it down as much as possible.

@mstahv
Copy link
Member

mstahv commented Mar 7, 2024

I know you are not the only one. Qualifying question, process involves also the front-end dependencies? (I can guess the answer based on your preferences, but just checkking)

@knoobie
Copy link
Contributor Author

knoobie commented Mar 7, 2024

Yes, that's why I'm excluding the react-components -> reduce SBOMs; which we have to publish in our internal artifactory for our security guys. (I have developed a custom maven plugin that creates a combined SBOM from npm + maven dependencies to show the "real" dependencies of vaadin and not just the maven stuff for auditing - similar to the new flow maven plugin goal; just a little bit more features..)

@Legioth
Copy link
Member

Legioth commented Mar 7, 2024

I suspect it would be difficult for users to understand the difference between com.vaadin:vaadin and com.vaadin:vaadin-platform dependencies. The "correct" name for the the vaadin dependency with the contents it had up until Vaadin 24.3 would really be com.vaadin:flow. Could we introduce that now and suggest in upgrade instructions to change to that dependency for those who don't plan to start using Hilla?

If we assume that this is the solution we would want to have in the long term, then it opens the question on whether it would really be worthwhile to have an intermediate solution for a while rather than just going for com.vaadin:flow right away?

@mstahv
Copy link
Member

mstahv commented Mar 7, 2024

There is no such a problem. People, especially the new ones, don't go to search.maven.org to look for dependecies they'd add to their app to add vaadin to it. And if they do, they are screwed already. New users take the modules we declared in the starter projects.

@knoobie
Copy link
Contributor Author

knoobie commented Mar 7, 2024

Probably off topic; just saying: In my opinion it's wrong to have everything in com.vaadin:vaadin - as example: spring boot also does not have everything in a single dependency. You wanna develop flow? Use the vaadin-flow-spring-boot-starter. You also wanna start using hilla and/or react - simply add vaadin-hilla-spring-boot-starter next to your flow starter. I would say consistency is key here - not simplicity (a single bundle of mud) because of the additional complexity and problem this creates all sides

It just feels wrong that I have to exclude Hilla from com.vaadin:vaadin in a Vaadin Quarkus or Jakarta EE app, so that I don't include Spring by accident

@Legioth
Copy link
Member

Legioth commented Mar 7, 2024

Semantically meaningful names matter in lots of situations. Otherwise, we could just name dependencies com.vaadin:a, com.vaadin:b and so on and in that way avoid spending lots of time discussing names.

com.vaadin:flow is nowadays much more semantically meaningful name for Vaadin Flow than what com.vaadin:vaadin is. The only reason to stick to the vaadin name is backwards compatibility. The big question is just where to draw the line between semantically meaningful names and backwards compatiblity.

@Legioth
Copy link
Member

Legioth commented Mar 7, 2024

The comparison with Spring Boot is interesting even though I don't think it's fully comparable since we don't aspire to even get close to the same breadth.

At the same time, Spring Boot also needs exclusions if you want to use Jetty instead of Tomcat so it's not like their approach is a panacea against application-level exclusions either.

@knoobie
Copy link
Contributor Author

knoobie commented Mar 7, 2024

"Simplified" version of a spontaneous suggestion:

  • com.vaadin:vaadin - the big ball of everything (not sure who needs it)
  • com.vaadin:vaadin-spring-boot-starter
    • com.vaadin:vaadin-flow-spring-boot-starter
    • com.vaadin:vaadin-hilla-spring-boot-starter
  • com.vaadin:vaadin-flow-spring-boot-starter
    • com.vaadin:vaadin-flow (includes flow + flow-core-components) (and pro?)
    • com.vaadin:vaadin-spring
  • com.vaadin:vaadin-hilla-spring-boot-starter
    • com.vaadin:vaadin-hilla (includes hilla + react)

Quarkus + Jakarta could just include vaadin-flow as migration.

@mstahv
Copy link
Member

mstahv commented Mar 7, 2024

With a bit of thinking, I'm quite sure this can be done in mostly backwards compatible manner.

@Legioth If you are switiching from tomcat to jetty with Spring Boot, you are replacing a required transitive dependency (which has a good default that most people are happy with) to an alternative. Quite different to shipping everything in one dependency because somebody somewhere might need them all. If Spring Boot would ship all dependencies by default, they would need Initializr only to choose between Maven and Gradle 😁

@Legioth
Copy link
Member

Legioth commented Mar 8, 2024

com.vaadin:vaadin - the big ball of everything (not sure who needs it)

I think that's the key to solving this puzzle. The vaadin and vaadin-core dependencies are really relics from an era before Spring Boot starters and Quarkus extensions entered the spotlight. Those dependences don't serve any useful purpose today and I suggest they would be deprecated without direct replacements.

As usual, we've got two different user categories to take into account. There's the new users who aren't yet familiar with exactly what the Vaadin platform is made up of and there's the veterans who know exactly what they want.

For the veterans, there could be separate Spring Boot Starters for Flow and Hilla respectively along with similar Flow-branded dependencies for Quarkus, CDI and vanilla Servlet. These wouldn't include any UI components so the user should also separately choose separate dependencies for Flow, React and/or Lit with our without the commercial components.

For new users, we would instead want to provide simplified single-dependency options tailored for the primary starting points: our own starting resources, Spring Initializer and the Quarkus extension directory. We want those dependencies to contain everything applicable so that new users can easily explore the possibilities. Our own starting points would use a single dependency with Spring Boot starters for both frameworks and all UI components for both Flow and React. 3rd party directories don't like to distribute commercial dependencies so Spring Initializer would have a similar dependency but without commercial components. Quarkus users would get a dependency with the extension for Flow and the open source Flow components. If we in the future make Hilla support Quarkus, then we would update this dependency to include Hilla and the open source React components whereas the other Quarkus extension intended for veterans would remain Flow-only.

Would this seem like a good vision for the future? (Let's for the moment not think about how this might impact the already delayed Vaadin 24.4 release schedule.)

@knoobie
Copy link
Contributor Author

knoobie commented Mar 8, 2024

Would this seem like a good vision for the future?

I would prefer this, but I'm probably not seeing the whole picture you guys are planning for the next months/years - as of now (and how it currently looks with 24.4); that would be a good solution for the current problems.

These wouldn't include any UI components so the user should also separately choose separate dependencies for Flow, React and/or Lit with our without the commercial components.

Wouldn't this also mean flow-react has to be optional? Like e.g. flow-components?

3rd party directories don't like to distribute commercial dependencies so Spring Initializer would have a similar dependency but without commercial components.

Interesting; this would also mean that flow-react has to be excluded by default? Already in 24.4, otherwise you could get de-listed from Spring Initializer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants