feat!: Make react-router default #18387
Conversation
Test Results1 055 files ± 0 1 055 suites ±0 1h 18m 15s ⏱️ + 3m 1s Results for commit 5ca2aae. ± Comparison against base commit 6627445. This pull request skips 1 test.♻️ This comment has been updated with latest results. |
| @@ -161,6 +161,9 @@ public class BuildDevBundleMojo extends AbstractMojo | |||
| @Parameter(property = "npm.postinstallPackages", defaultValue = "") | |||
| private List<String> postinstallPackages; | |||
|
|
|||
| @Parameter(property = InitParameters.REACT_ROUTER_ENABLED, defaultValue = "true") | |||
| private boolean reactRouterEnabled; | |||
There was a problem hiding this comment.
Are there any public available discussions about the benefits or downsides from this change for flow developers? Increased package size? Additional dependency on production to react? Performance downside or improvements? Customized index.html still applied?
There was a problem hiding this comment.
It's not a discussion per se, but there is this acceptance criteria: vaadin/platform#4710. I believe the net effects should be neutral in terms of performance and package sizes. The main benefit is enabling mixing Flow and Hilla/React views in the same app. For pure-Flow projects, the change should be neutral.
There was a problem hiding this comment.
Understandable, I was just wondering because the default is swapped in this PR (opposite as described in the AC) and therefore it would affect all flow apps. This is why I was wondering what the benefits or downsides are for flow only users :) (except to get rid of the "abandoned" vaadin router in favor of the react router)
There was a problem hiding this comment.
Increased package size? Additional dependency on production to react?
Good point and we're going to estimate the bundle size. I don't expect much extra comparing to previous default, but let's see.
Customized index.html still applied?
Yes, I expect so, haven't tested yet though. Let me add a note about it.
Enable react router as default with property to switch back to vaadin router.
caalador
force-pushed
the
feat/react-router-default
branch
from
4664d9e
to
b7d5007
Compare
caalador
force-pushed
the
feat/react-router-default
branch
from
1ae814c
to
f133954
Compare
|
caalador
force-pushed
the
feat/react-router-default
branch
7 times, most recently
from
32c5b35
to
61010d5
Compare
caalador
force-pushed
the
feat/react-router-default
branch
from
61010d5
to
1006361
Compare
caalador
force-pushed
the
feat/react-router-default
branch
7 times, most recently
from
d6d864b
to
3876bec
Compare
caalador
force-pushed
the
feat/react-router-default
branch
from
3876bec
to
457b033
Compare
caalador
force-pushed
the
feat/react-router-default
branch
from
2b15d91
to
b3657a8
Compare
caalador
force-pushed
the
feat/react-router-default
branch
from
4c30d0b
to
9da1a67
Compare
There was a problem hiding this comment.
Should reactRouterEnabled also be an input parameter?
There was a problem hiding this comment.
I don't know. Basically it does change the package.json contents so perhaps?
…to feat/react-router-default
caalador
force-pushed
the
feat/react-router-default
branch
from
9a9804f
to
bac5a2a
Compare
|
* feat!: Make react-router default Enable react router as default with property to switch back to vaadin router. * No react for test modules with explicit vaadin-router import * Add react state also for dev-server init * add optional param to not get can not navigate * Handle hash anchor update * Handle popstate without router navigation * pick first popstate * fix test excpectation * Add react router flag as input in gradle * fix typo * Fix outside navigation * Fix server webcomponent init --------- Co-authored-by: Peter Czuczor <61667986+czp13@users.noreply.github.com>
|
This ticket/PR has been released with Vaadin 24.4.0. |
Enable react router as default with
property to switch back to vaadin router.