Router manager queue #3540

grimasod · 2019-09-13T08:43:34Z

Related issues

closes #3454

Short description and why it's useful

Efficiently add routes to Vue Router, with optional priority for theme routes

Screenshots of visual changes before/after (if there are any)

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

No upgrade steps required (100% backward compatibility and no breaking changes)
I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

I read and followed contribution rules

update slack invitation link

…into feature/router-manager-queue # Conflicts: # core/lib/router-manager.ts

grimasod · 2019-09-13T08:44:15Z

Still in progress

pkarw · 2019-09-13T11:59:53Z

Cool, looks like a step into right direction!

grimasod · 2019-09-19T07:11:48Z

i'm not sure about this idea - ... - so the question is what for is this change?

I found inconsistencies causing routes to be registered twice. Specifically in Category and Product Vuex actions, they register mappings with localizedDispatcherRoute, which adds a leading slash. It was easier to add the one exception for mappingFallback. And routes need the '/ when adde to the router. But if this causes other problems, or you think it's a bad idea, I can change the catalog actions instead.

grimasod · 2019-09-19T07:16:58Z

We have to do one or the other. Without any change there's a mismatch when the page is reloaded on a category or product page and nothing is loaded

patzick · 2019-09-19T07:21:34Z

@grimasod could you merge the master branch into this branch also? I started to merge master-> develop and I have multiple conflicts around route manager. And you provided more changes here so I think best way would be to merge master here, test everything with QA and merge into develop as a final solution. What do you say? I'll do CR right after merge if you would let me know that it's done :)

grimasod · 2019-09-19T07:41:26Z

All tested...

checking if url/mappingFallback is not being called (debugger or logger) - to make sure registerMappings still works just fine

Tested - mappingFallback only runs when the initial page load is a category or product page

checking swithcing the URL between multistore instances (/de -> /it based urls etc)

Tested - all multistore urls load correctly

checking in the console for hydration errors

Tested - there are the same issues on Category and Products pages as the Develop branch

checking the route not found errors in the console

Tested - no errors reported

page not found,

Tested - default, IT, DE, all work correctly

if it;s possible to check if the routes are not duplicated (debugger or log)

Tested - both client and server appear correct, with no duplication

check the number of addRoutes call to make sure this optimization gives us proper value

Tested - addRoutes runs once on server and once on client during initial page load. Then once when each new dynamic route is opened for the first time

grimasod · 2019-09-19T08:12:07Z

@patzick I can try, after we resolve the last issue.

I have merged Develop into this branch every day, with all the other changes that have been made there. I also combined the 1.10.3 router changes into this branch, so hopefully this has all the updates already

patzick · 2019-09-19T11:59:16Z

@grimasod great, that's why I'd like to do this thru your branch as it contains the most accurate final version :)

pkarw · 2019-09-19T13:26:52Z

Hey @grimasod thanks for the tests you executed; looking cool; when it will be ready for merge?

…into feature/router-manager-queue

…into feature/router-manager-queue # Conflicts: # CHANGELOG.md # core/client-entry.ts # core/i18n/package.json # core/lib/multistore.ts # core/lib/router-manager.ts # core/modules/catalog/store/category/actions.ts # core/modules/catalog/store/product/actions.ts # core/modules/url/helpers/index.ts # core/modules/url/router/beforeEach.ts # core/modules/url/store/actions.ts # core/server-entry.ts # docs/package.json # lerna.json # package.json # src/themes/default-amp/package.json # src/themes/default/components/core/blocks/SearchPanel/SearchPanel.vue # src/themes/default/components/core/blocks/SidebarMenu/SidebarMenu.vue # src/themes/default/components/theme/blocks/AsyncSidebar/AsyncSidebar.vue # src/themes/default/package.json # src/themes/default/resource/banners/de_promoted_offers.json # test/unit/package.json

grimasod · 2019-09-20T08:26:12Z

@patzick I've merged Master. I hope I got it right! :)
@pkarw It's ready now.

I realized the multistore route names do need the store prefix, like "it-home", so they can be customized in the theme route definitions when not using appendStoreCode

pkarw · 2019-09-20T09:30:57Z

Cool! Thanks! @patzick and @andrzejewsky will take care of CR and tests along with @alinadivante - it’s pretty crucial change

alinadivante · 2019-09-20T09:34:59Z

Ok, I've already started testing this branch :) There is the same problem which I reported here #3579
@andrzejewsky will handle these errors

alinadivante · 2019-09-20T10:35:11Z

There is a small problem with /page-not-found
I see the difference between Server-side and Client-side..
go to http://localhost:3000/it and click on About us (Magento CMS) link from footer - - the page was not found.
Now refresh http://localhost:3000/it/i/about-us and you will see that pictures from Visualizza i prodotti più venduti disappear

alinadivante · 2019-09-20T11:49:02Z

Second problem regarding differences between server side and client side:

Open http://localhost:3000
Log in
Go to my profile
Log out - I'm redirected to Home page and logged out correctly
now:
Open http://localhost:3000
Log in
Go to my profile
Refresh the page
Log out - I'm redirected to Home page, but I'm not logged out

patzick

Everything here works well, really nice job @grimasod! To not hold this PR anymore, QA issues will be addressed in separate PR. Thanks! :)

patzick · 2019-09-21T18:58:42Z

core/lib/multistore.ts

@@ -196,9 +195,14 @@ export function adjustMultistoreApiUrl (url: string): string {
 }

 export function localizedDispatcherRoute (routeObj: LocalizedRoute | string, storeCode: string): LocalizedRoute | string {
-  const appendStoreCodePrefix = config.storeViews[storeCode] ? config.storeViews[storeCode].appendStoreCode : false
+  const { storeCode: currentStoreCode, appendStoreCode } = currentStoreView()
+  if (!storeCode || !config.storeViews[storeCode]) {


i've fixed tests mostly by adding here || !config.storeViews[storeCode]. It prevents us from switching to not existing storeView also

…#3540)

kkdg and others added 16 commits September 11, 2019 21:10

[Docs] Updated cookbook/theme Intro

00a4971

[Docs] Updated cookbook/theme R0 - blueprint done

9ac6f40

[Docs] Updated cookbook/theme R4

0084fb3

[Docs] Updated cookbook/theme R1 - secret outline

55e8596

[Docs] Updated core-themes/themes - fixing _point_ for clarify

3b07c80

update slack invitation link

16cde8b

Update README.md

781aabe

[Docs] Updated cookbook/theme R1 - intro

45f3457

Merge pull request vuestorefront#3534 from DivanteLtd/filrak-patch-1

7eb04b6

update slack invitation link

[Docs] Updated cookbook/theme R1 - recipe ideation

fd7717f

[WIP] RouterManager route queue

ce94124

Merge branch 'develop' of https://github.com/DivanteLtd/vue-storefront …

3fd455d

…into feature/router-manager-queue # Conflicts: # core/lib/router-manager.ts

[WIP] RouterManager fix find in queue

b2f6776

Fix multistore localizedRoute when used by mixin

2625299

mulitstore disatcher check param

6fbc97e

Router Manager add routes by priority

839dea6

pkarw added the not ready for merge PR is holded. Needs some clarifications or things that need to be finished. label Sep 13, 2019

kkdg and others added 11 commits September 14, 2019 15:21

[Docs] Updated cookbook/migration config added

2f38c14

[Docs] Updated cookbook/migration R0 done

26ea7aa

[Docs] Upadted cookbook - fix typos

2dd328a

[Docs] Updated cookbook/migration R0, R1

e5dbeac

[Docs] Updated cookbook/migration R0 - intro update

639eb25

[Docs] Updated cookbook/migration R0 - remove ambiguity

acaf8e7

[Docs] Updated cookbook/theme ...

5c797eb

Fixed menu sidebar overflow-x

b5eeab2

Added smooth scroll

056376d

Updated changelog

7619686

Smooth scrolling for sidebar menu

ec687d3

pkarw approved these changes Sep 19, 2019

View reviewed changes

David Grimason added 4 commits September 20, 2019 10:10

Merge branch 'develop' of https://github.com/DivanteLtd/vue-storefront …

3e2da1b

…into feature/router-manager-queue

Changelog and docs for 3540

a73b4a3

Router manager - reinstate prefixed routes names, so can be customized

fc4476c

grimasod changed the title ~~[WIP] Router manager queue~~ Router manager queue Sep 20, 2019

patzick added the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label Sep 20, 2019

pkarw mentioned this pull request Sep 21, 2019

Refactoring storeCodeFromRoute, resolve store code on SSR #3593

Merged

6 tasks

fallback to current storeCode when chosen not found, fixed tests

78c170f

patzick approved these changes Sep 21, 2019

View reviewed changes

patzick merged commit 64d4631 into vuestorefront:develop Sep 21, 2019

This was referenced Sep 22, 2019

Fix loading bestsellers #3599

Merged

Bestsellers are not loading properly on "404" page. #3594

Closed

pkarw mentioned this pull request Sep 24, 2019

wait for translations messages while loading app #3605

Merged

6 tasks

pkarw added a commit that referenced this pull request Sep 25, 2019

proper 404 routing reversed (it was missing ddue to master merge in to …

d2ea3b9

…#3540)

grimasod deleted the feature/router-manager-queue branch September 26, 2019 00:49

alinadivante added QA approved after merge Testers will add this label after positive check on merged changes and removed QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. labels Oct 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Router manager queue #3540

Router manager queue #3540

grimasod commented Sep 13, 2019 •

edited

Loading

grimasod commented Sep 13, 2019

pkarw commented Sep 13, 2019

grimasod commented Sep 19, 2019

grimasod commented Sep 19, 2019

patzick commented Sep 19, 2019

grimasod commented Sep 19, 2019

grimasod commented Sep 19, 2019

patzick commented Sep 19, 2019

pkarw commented Sep 19, 2019

grimasod commented Sep 20, 2019

pkarw commented Sep 20, 2019

alinadivante commented Sep 20, 2019

alinadivante commented Sep 20, 2019

alinadivante commented Sep 20, 2019 •

edited by ArturDivante

Loading

patzick left a comment

patzick Sep 21, 2019

Router manager queue #3540

Router manager queue #3540

Conversation

grimasod commented Sep 13, 2019 • edited Loading

Related issues

Short description and why it's useful

Screenshots of visual changes before/after (if there are any)

Which environment this relates to

Upgrade Notes and Changelog

Contribution and currently important rules acceptance

grimasod commented Sep 13, 2019

pkarw commented Sep 13, 2019

grimasod commented Sep 19, 2019

grimasod commented Sep 19, 2019

patzick commented Sep 19, 2019

grimasod commented Sep 19, 2019

grimasod commented Sep 19, 2019

patzick commented Sep 19, 2019

pkarw commented Sep 19, 2019

grimasod commented Sep 20, 2019

pkarw commented Sep 20, 2019

alinadivante commented Sep 20, 2019

alinadivante commented Sep 20, 2019

alinadivante commented Sep 20, 2019 • edited by ArturDivante Loading

patzick left a comment

Choose a reason for hiding this comment

patzick Sep 21, 2019

Choose a reason for hiding this comment

grimasod commented Sep 13, 2019 •

edited

Loading

alinadivante commented Sep 20, 2019 •

edited by ArturDivante

Loading