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: Pass route data through window.Vaadin.routesConfig instead of an import #19666

Merged
merged 10 commits into from
Jul 4, 2024

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Jul 3, 2024

Instead of importing routes.js/routes.ts/routes.tsx from the project into Flow.tsx and creating a circular dependency, this moves importing of routes.js to a new file that is always included when react is used vaadin-react.tsx. This file publishes the routes as window.Vaadin.routesConfig so that Flow.tsx can read them from there without directly depending on routes.js

Fixes #19658

@Artur- Artur- force-pushed the routes-through-global branch from acc4061 to 4a11fa2 Compare July 3, 2024 14:38
Copy link

github-actions bot commented Jul 3, 2024

Test Results

1 127 files  ±0  1 127 suites  ±0   1h 24m 29s ⏱️ -3s
7 297 tests ±0  7 247 ✅ ±0  50 💤 ±0  0 ❌ ±0 
7 624 runs   - 8  7 569 ✅  - 3  55 💤  - 5  0 ❌ ±0 

Results for commit fbccb31. ± Comparison against base commit c31e8cd.

♻️ This comment has been updated with latest results.

@Artur- Artur- marked this pull request as ready for review July 3, 2024 17:44
@Artur- Artur- requested a review from mshabarov July 4, 2024 07:40
Copy link

sonarqubecloud bot commented Jul 4, 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.

Tested the hmr for react views and server-side live reload with the hybrid app - works as expected. The codes looks good as well.

@mshabarov mshabarov merged commit 9b88c4c into main Jul 4, 2024
25 of 26 checks passed
@mshabarov mshabarov deleted the routes-through-global branch July 4, 2024 10:46
vaadin-bot pushed a commit that referenced this pull request Jul 4, 2024
… import (#19666)

Instead of importing routes.js/routes.ts/routes.tsx from the project into Flow.tsx and creating a circular dependency, this moves importing of routes.js to a new file that is always included when react is used vaadin-react.tsx. This file publishes the routes as window.Vaadin.routesConfig so that Flow.tsx can read them from there without directly depending on routes.js

Fixes #19658
vaadin-bot added a commit that referenced this pull request Jul 4, 2024
… import (#19666) (#19670)

Instead of importing routes.js/routes.ts/routes.tsx from the project into Flow.tsx and creating a circular dependency, this moves importing of routes.js to a new file that is always included when react is used vaadin-react.tsx. This file publishes the routes as window.Vaadin.routesConfig so that Flow.tsx can read them from there without directly depending on routes.js

Fixes #19658

Co-authored-by: Artur <[email protected]>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.5.0.alpha4 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.

Circular imports in routes.tsx + Flow.tsx
3 participants