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

RouteConfigService pull url matcher. #72

Open
morgnism opened this issue Dec 16, 2021 · 5 comments
Open

RouteConfigService pull url matcher. #72

morgnism opened this issue Dec 16, 2021 · 5 comments
Labels
feature New Issue route-config @this-dot/route-config package related issues

Comments

@morgnism
Copy link

Is your feature request related to a problem? Please describe.

RouteConfigService allows me to access the data property either from path object | Resolver but doesn't yet access params from a custom route matcher, for example.

Describe the solution you'd like

If using a custom matcher to return a new UrlSegment, that value is stored in params and accessed via ParamMap. I'd like to be able to either:

  1. access params observable like with ActivatedRoute#ParamsMap
  2. OR have the same service as RouteConfigService
  3. OR a new service other than RouteConfigService

Describe alternatives you've considered

My specific use case required accessing resolved data from a parent route and accessing the returned UrlSegment from the route matcher then performing a withLatestFrom to map the params from ActivatedRoute:

a$ = this.routeConfigService.getLeafConfig('someValue').pipe(
      withLatestFrom(
        this.route.paramMap.pipe(map((params: ParamMap) => params.get('path'))), // 'path' is the UrlSegment
      ).pipe(...)
)

constructor(
  private route: ActivatedRoute,
  private routeConfgiService: RouteConfigService<string, 'someValue'>
) {}

Addtional context

No response

@TapaiBalazs TapaiBalazs added good first issue Good for newcomers and removed good first issue Good for newcomers labels Dec 17, 2021
@TapaiBalazs TapaiBalazs added the route-config @this-dot/route-config package related issues label Jan 14, 2022
@morgnism
Copy link
Author

morgnism commented Oct 3, 2022

For the proposal, make a POC where this is an operator that handles the edge case and is reusable:

withLatestFrom(
    this.route.paramMap.pipe(map((params: ParamMap) => params.get('path'))), // 'path' is the UrlSegment
).pipe(...)

@morgnism
Copy link
Author

morgnism commented Oct 28, 2022

Extend the RouteConfigService into a new service for combining the data from the resolver with the result from a route's URL matcher.

Requirements:

  • Provide a new service with this feature
  • Unit test the service
  • Provide examples in the Readme and comments

How long will it take to build? What milestones exist (if applicable)?

Milestone 1 - MVP service for URL matching (major version bump 1.3.0)

  • Create a new service that combines resolver leaf data from RouteConfigService with the URL segment from a route matcher.
  • Extend unit tests for the combined path (may need help from maintainers).

Est time: 4 hours remaining after the POC

@morgnism
Copy link
Author

Comments from Chris:

This looks like a good direction. However, as far as I can see the current POC is a breaking change to the current API.
IMO the best approach for now would be to put that logic in a separate service and make that other service combine the data from the current getActivatedRouteConfig with the new data from the paramMap
Additionally, I noticed that you only take the paramMap from the “leaf” activated route. This means that if you had couple of nested routes and each of them had a custom route matcher (that only consumed part of the path) you would only have access to paramMap from the last matcher. If you look at this logic it actually takes all the activated routes from the current one to the root and merges all of the values into one object. I think having similar logic for paramMap would be beneficial.
We could also just add new methods to the current service but the main objective is to not introduce breaking changes so all the features would need to be added via new methods IMO.
Oh… and one final suggestion. With all of the above in mind I think we should use combineLatest the merge the “data” and “paramMap” properties. This makes sure that the data would always be fresh.

@morgnism
Copy link
Author

This work should be followed with a blog post about the new service.

This post should include:

  • what new features were added
  • what you can do with these features (short code examples)

@dustinsgoodman
Copy link
Contributor

@morgnism is the estimate based on the issues Chris raised in the POC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New Issue route-config @this-dot/route-config package related issues
Projects
None yet
Development

No branches or pull requests

3 participants