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

feat: add endpoint detection #20567

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

feat: add endpoint detection #20567

wants to merge 15 commits into from

Conversation

tepi
Copy link
Contributor

@tepi tepi commented Nov 27, 2024

Add check for if endpoint generator needs to run

Fixes #20289
Fixes #18800

Copy link

github-actions bot commented Nov 27, 2024

Test Results

1 159 files  ±0  1 159 suites  ±0   1h 32m 47s ⏱️ - 2m 37s
7 583 tests +2  7 527 ✅ +2  56 💤 ±0  0 ❌ ±0 
7 928 runs   - 5  7 865 ✅  - 3  63 💤  - 2  0 ❌ ±0 

Results for commit 281f16a. ± Comparison against base commit bcd7a0d.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
com.vaadin.flow.server.frontend.NodeTasksHillaTest ‑ should_useHillaEngine_whenEnabled
com.vaadin.flow.server.frontend.NodeTasksHillaTest ‑ should_notUseHillaEngine_whenEndpointsAreNotDetected
com.vaadin.flow.server.frontend.NodeTasksHillaTest ‑ should_useHillaEngine_whenEndpointsAreDetected
com.vaadin.flow.server.frontend.NodeUpdaterTest ‑ getDefaultDependencies_endpointsAreUsed_addsHillaPackages

♻️ This comment has been updated with latest results.

@tepi
Copy link
Contributor Author

tepi commented Dec 4, 2024

For reviewers to test this:

  1. You need local clones of flow, hilla and platform repos
  2. For Flow checkout this branch, for Hilla checkout this branch, for Platform checkout main branch
  3. Build flow: mvn clean install -DskipTests
  4. Build hilla: npm run build , then mvn clean install -DskipTests
  5. Build vaadin maven plugin: mvn clean install -DskipTests in platform/vaadin-maven-plugin
  6. After that test with a project from start, using 24.6-SNAPSHOT as the vaadin version

@tepi
Copy link
Contributor Author

tepi commented Dec 5, 2024

@mshabarov When you have time could you check that it still works for your use case with the latest change?

mshabarov
mshabarov previously approved these changes Dec 5, 2024
Copy link
Contributor

@mshabarov mshabarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test manually with the following scenarios:

  1. Vaadin app has only Java views and a ReactAdapter
  2. Vaadin app has only TS view
  3. Vaadin app has a TS view and a ReactAdapter.

Works as expected. LGTM.

@mshabarov
Copy link
Contributor

⚠️ Let's not merge this until we get a green light from Hilla team for vaadin/hilla#2938.

@mshabarov mshabarov marked this pull request as draft December 10, 2024 11:59
@mshabarov mshabarov force-pushed the feat/endpoint-detection branch from 7ca001f to 9a1c149 Compare December 19, 2024 12:40
@mshabarov mshabarov changed the base branch from 24.6 to main December 19, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants