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

fix: React file not added when reactEnabled false #19845

Merged
merged 3 commits into from
Aug 30, 2024

Conversation

caalador
Copy link
Contributor

@caalador caalador commented Aug 28, 2024

Do not add the outlet
file if react is not
enabled as the required
react node files are not
available.

Fixes #19842
Fixes vaadin/start#3159

Copy link

github-actions bot commented Aug 28, 2024

Test Results

1 135 files  ± 0  1 135 suites  ±0   1h 28m 20s ⏱️ - 1m 12s
7 381 tests ± 0  7 331 ✅ ± 0  50 💤 ±0  0 ❌ ±0 
7 750 runs  +32  7 690 ✅ +32  60 💤 ±0  0 ❌ ±0 

Results for commit 8dae0a3. ± Comparison against base commit e71b834.

♻️ This comment has been updated with latest results.

Do not add the outlet
file if react is not
enabled as the required
react node files are not
available.

Fixes #19842
Copy link

sonarcloud bot commented Aug 29, 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.

A unit test would be good to have, but it probably require efforts to mock things that are mostly private.

@@ -579,7 +579,8 @@ public static boolean validateLicenses(PluginAdapterBase adapter) {
}

FrontendDependenciesScanner scanner = new FrontendDependenciesScanner.FrontendDependenciesScannerFactory()
.createScanner(false, adapter.getClassFinder(), true, null);
.createScanner(false, adapter.getClassFinder(), true, null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thought about replacing two calls to adapter by just passing adapter object, but scanner probably shouldn't be coupled to adapter, as it can be called from context where it's not available.

@mshabarov mshabarov merged commit 3dc38a6 into main Aug 30, 2024
25 of 26 checks passed
@mshabarov mshabarov deleted the issues/19842-react-false branch August 30, 2024 06:06
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.5.0.alpha13 and is also targeting the upcoming stable 24.5.0 version.

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

Successfully merging this pull request may close these issues.

ReactRouterOutletElement.tsx breaks build of Flow application using vaadin-router
3 participants