Feature/2010 seo friendly urls dynroutes

[pkarw]

Related issues

Related to #2401 - second approach

Short description and why it's useful

(describe in a few words what is this Pull Request changing and why it's useful)

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

(if you made any changes in the UI layer please provide before/after screenshots)

Screenshot of passed e2e tests (if you are using our standard setup as a backend)

(run yarn test:e2e and paste the results. If you are not using our standard backend setup or demo.vuestorefront.io you can ommit this step)

Upgrade Notes and Changelog

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

Contribution and currently important rules acceptance

• I read and followed contribution rules
• I read the TypeScript Action Plan and adjusted my PR according to it
• I read about Vue Storefront Modules and I am aware that every new feature should be a module

[filrak]

This one looks much better and less complicated at first glance! I still need time to dig deeper into it tomorrow to understand the details but overally it looks like the right way to do this :)

The only thing that concerns me is there might be better to put all of this stuff in one VS module. I guess if we want this feature to be optionally it's good to keep it in a separate threeshakable module so people that don't use it are not including the unused code into the bundle. It also helps very much with understanding and maintenanc since everything regarding this feature is in one place.

Even if we are adding something to the function directly (like config checks)

 computed: {
    productLink () {
      return ((this.$store.state.config.seo.useUrlDispatcher) ? 
        this.localizedDispatcherRoute({
          fullPath: this.$store.state.config.seo.useUrlDispatcher ? this.product.url_path : null,
          params: {
            childSku: this.product.sku === this.product.parentSku ? null : this.product.sku
          }
        })
      :
        this.localizedRoute({
          name: this.product.type_id + '-product',
          params: {
            parentSku: this.product.parentSku ? this.product.parentSku : this.product.sku,
            slug: this.product.slug,
            childSku: this.product.sku
          }
        })
      )   
    },  

maybe it's still better to keep this logic inside module and just import this function as a helper or something else.

With this the separation of concerns is kept and in case of lack of the module we are only gently warned about failed import instead of being forced to debug code and figure out which part is responsible for the module that we don't have?

So instead of above code it may look like:

import { CustomURLProductLink } from '@vue-storefront/core/modules/seo-priendly-url/helpers'
// later in code
 computed: {
    productLink: 
      CustomURLProductLink(this) :||
      // regular code if module is not working
 },  

I guess it may be a good general approach for such things in the project. Wdyt?

// Edited

[pkarw]

Thanks Filip :) Yea that might have sense however in that case url module always must be loaded it’s no longer optional. Let’s think this through. The only problem I’ve currently got with this approach is csr/ssr hydration issue :( no idea why :(

[filrak]

maybe the hydration check is fired too early ( @patzick tried to make this check too and it didn't worked as intended ). afaik there was a library allowing to delay hydration (it's in some of recent vue newsletters)

Btw i figured out how to deal with removing this config checks and encapsulating all of the logic in the imported function.

import { CustomURLProductLink } from '@vue-storefront/core/modules/seo-friendly-url/helpers'
// later in code
 computed: {
    productLink: 
      this.$store.state.config.seo.useUrlDispatcher ? CustomURLProductLink(this) : 
      // regular code if module is not working
 },

we can just use OR logical operator since it always return the value of part that returned true instead of true so it can work like:

 computed: {
    productLink: CustomURLProductLink(this) || // regular code if module is not working
 },

if CustomURLProductLink will throw false it will return value of the second function so we basically need to check the config param inside function

CustomURLProductLink (context) {
 if (this.$store.state.config.seo.useUrlDispatcher) {
  // logic if the config param is present
 } else {
   return false
 }
}

[filrak]

Yea that might have sense however in that case url module always must be loaded it’s no longer optional

why?

[pkarw]

@filrak OK, i got it - so even module won't be loaded the helper could be used

[pkarw]

TODO:

• code changes [WIP] PoC - multipart, dynamic URL rewrite #2401 - for example parse-query
• url helper in favor of "ifology"
• tests
• error handling tests
• overall tests with the config.seo.urlDispatcher=false

[pkarw]

FIXED:

• SSR support,
• references to config.seo.useUrlDispatcher limited just to url module

TODO:

• multistore routing
• multistore links

[patzick]

Pretty huge PR :)
i've made ~half of it and i'll finish it tomorrow, but i give what i have for now

(Note to myself: finished on core/modules/catalog/store/product/actions.ts)

[pkarw]

OK, It's ALL DONE!

82 comments; 58 files modified, 59 commits -> wow it became one of the biggest CRs processed so far haha :)

[filrak]

Thanks for correcting!

[patzick]

Okay, i think we can merge this now.

@pkarw please apply just suggestions and resolve conflicts - nice job here! This is gonna be a cool feature!

[pkarw]

@patzick OK, done!

[pkarw]

Thanks, done

[Zyles]

Thanks for working on this. We want to go full headless but we can't until we know a storefont framework that can support the same URLs that Magento 2 itself generates. Because we don't want /p/ /c/ etc. in URLs or anything else like identifiers. Because we don't want to 301 redirect thousands of existing URLs.

Will this new update support product urls with "/my-product.html" and "/my-category/sub-category/" style? We use both, ".html" for products and "/" suffix for category/page.

Does it also support language code in url? "mydomain.com/fr/my-product.html" ?

Thanks.

[pkarw]

@Zyles correct. With this feature - starting from 1.9.0-rc1 you'll be able to set the URL structure of your choice just using the product.url_path and category.url_path fields