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

Spike: SEO-friendly URLs (ISREST-925,#33) #11

Closed
wants to merge 6 commits into from

Conversation

fmalcher
Copy link
Collaborator

@fmalcher fmalcher commented Nov 21, 2019

This PR prepares custom URL formats for products and categories.
It consists of two parts:

  1. Provide custom matchers for product and category routes that define the same URL format like before. Thus, this is non-breaking and could be merged. ⚠️ Important for review: Look at the new structure!

  2. Define a second implementation for the matchers (and URL generators) to provide a different URL pattern. Caution: No tests added here because this is just a concept proof. Once we agreed on a URL format, we should also add tests for the matcher.

http://localhost:4200/Cameras-Camcorders.832-c
http://localhost:4200/Cameras-Camcorders.832-c/Google-Home-p-201807171.html
http://localhost:4200/Google-Home-p-201807171.html

TODO:

  • Agree on a custom URL format
  • Implement it
  • Add tests for the matcher and generator function
  • Find out if there are places where URLs are still hard coded
  • Add documentation on how our matcher format works
  • Squash branch since the commit history is not clean

Notes for documentation

TODO: Find a better place for this!

  • use a custom matcher when the URL can't be matched by the simple standard path format
  • set the matcher in the route via matcher property
  • for each custom route format create an object of type CustomRoute.
  • CustomRoute contains the matcher, a function for generating route URLs and all possible matched formats
    • matcher: used in the route
    • URL generator: used for creating URLs based on entities, used in RouterLink and router.navigateByUrl()
    • format list: used in effects to match the route

Matcher function

  • of type UrlMatcher by Angular
  • Arguments:
    • array of UrlSegments (parts of the URL separated by slashes)
    • the route
  • matcher must use its arguments to decide whether the route will match or not
  • matcher must set route.data to empty object of no route data is defined
  • on no match: return null/undefined
  • on match:
    • set route.data.format to the actually matched route format, e.g. product/:sku/**. Also add this format to the list of all possible formats in the CustomRoute object
    • return object with properties:
      • posParams: all extracted params from the route, like sku or categoryUniqueId. Each as instance of UrlSegment
      • consumed: the part of the path that was covered by the matcher. This is necessary for when a sub-module contains other routes. Usually not interesting in our scenarios, thus returns the whole array of UrlSegments (url) in most cases

@fmalcher fmalcher changed the title feat: add custom url matchers for product and category route SEO-friendly URLs (ISREST-925) Nov 21, 2019
@fmalcher fmalcher changed the title SEO-friendly URLs (ISREST-925) feat: SEO-friendly URLs (ISREST-925) Nov 21, 2019
@fmalcher fmalcher force-pushed the feature/ISREST-925_SEO_friendly_URLs branch from f44e1b9 to b570038 Compare November 21, 2019 12:56
@fmalcher fmalcher marked this pull request as ready for review November 21, 2019 15:00
@fmalcher fmalcher self-assigned this Nov 21, 2019
@fmalcher fmalcher force-pushed the feature/ISREST-925_SEO_friendly_URLs branch 2 times, most recently from fa57f95 to ccb78ab Compare November 25, 2019 14:21
@fmalcher
Copy link
Collaborator Author

fmalcher commented Nov 25, 2019

@shauke pointed out that ofRoute() also uses the route path. Things got a bit tricky here: The ofRoute() mechanics use the path property from the route to determine the full path – but we don't have this when using a matcher.

I needed to make the route (!) contain the actually matched path format. Since the matcher decides this on its own we don't know this before the matcher has run.
The only place where we could reliably store such information is the route data. Thus, the matcher now writes its decision to the route data, and the ofRoute() operator uses this data to get the full path.

I also introduced a CustomRoute object (+ interface) that contains matcher, URL generator and possible formats. This list of formats can be used in effects then to match with ofRoute().

Possible TODO:
Generalize this a bit more! The route formats are duplicated now: once in the matcher, once in the list of possible formats.

@shauke shauke added the feature New feature or request label Dec 3, 2019
@rkarl-ish rkarl-ish changed the title feat: SEO-friendly URLs (ISREST-925) feat: SEO-friendly URLs (ISREST-925,#33) Dec 11, 2019
@rkarl-ish rkarl-ish mentioned this pull request Dec 11, 2019
@shauke shauke force-pushed the feature/ISREST-925_SEO_friendly_URLs branch from ccb78ab to c306aa7 Compare December 11, 2019 19:07
@shauke shauke force-pushed the feature/ISREST-925_SEO_friendly_URLs branch 2 times, most recently from c83093e to f5db0f2 Compare December 12, 2019 15:29
@shauke shauke assigned dhhyi and unassigned fmalcher Dec 13, 2019
@Sebastian-Haehnlein Sebastian-Haehnlein force-pushed the feature/ISREST-925_SEO_friendly_URLs branch from f5db0f2 to 09cba8e Compare January 9, 2020 08:37
@Sebastian-Haehnlein Sebastian-Haehnlein force-pushed the feature/ISREST-925_SEO_friendly_URLs branch from 09cba8e to f1204c2 Compare January 9, 2020 08:58
@Sebastian-Haehnlein Sebastian-Haehnlein force-pushed the feature/ISREST-925_SEO_friendly_URLs branch from f1204c2 to 17b617f Compare January 9, 2020 10:18
@dhhyi dhhyi added the wait Waiting for something (e.g. new ICM release) label Jan 20, 2020
@dhhyi
Copy link
Collaborator

dhhyi commented Jan 20, 2020

waiting for possible interaction with mapping library (https://github.com/intershop/intershop-pwa/projects/2#card-31666961)

@dhhyi dhhyi mentioned this pull request Feb 11, 2020
@dhhyi dhhyi changed the title feat: SEO-friendly URLs (ISREST-925,#33) Spike: SEO-friendly URLs (ISREST-925,#33) Feb 11, 2020
@dhhyi
Copy link
Collaborator

dhhyi commented Feb 16, 2020

Ideas integrated into #110

@dhhyi dhhyi closed this Feb 16, 2020
@dhhyi dhhyi deleted the feature/ISREST-925_SEO_friendly_URLs branch February 16, 2020 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request wait Waiting for something (e.g. new ICM release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants