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

feat(external-router)!: rework to use canMatchFn #2907

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

damienwebdev
Copy link
Member

@damienwebdev damienwebdev commented Jul 11, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

Fixes: #1855
Fixes: #1672

What is the current behavior?

Currently, developers have to use DaffExternalRouterExistenceGuard which requires:

  • Extraneous wildcard routing configuration
{
    path: '**',
    canActivate: [DaffExternalRouterExistenceGuard],
    children: [],
  },
  • Extraneous routing actions when the wildcard is reached. If you enable your Angular routing debug log, you would see:
Navigate (id:1)
NavigateCancel (id:1)
Navigate (id:2)
Navigated (id:2)

when navigating to any route that requires use of this guard alongside daffPath data.

  • Complicated "insertion strategies" to inform Angular's routing configuration of the dynamic routes.
DaffExternalRouterModule.forRoot({
      failedResolutionPath: 'error',
      notFoundResolutionPath: 'not-found',
    }, [
      {
        type: 'CATEGORY',
        insertionStrategy: daffInsertDataPathStrategy,
        route: {},
      },
      {
        type: 'PRODUCT',
        insertionStrategy: daffInsertDataPathStrategy,
        route: {},
      },
    ]),
  • Complicated dynamic route configuration
{
        matcher: daffDataPathUrlMatcher,
        data: {
          daffExternalRouteType: 'PRODUCT',
        },
        component: ProductComponent
},

What is the new behavior?

The new behavior leverages the canMatch guards created in Angular 14. You can configure routes as follows (without any complex looking configuration):

import { ApplicationConfig } from '@angular/core';
...
import { provideExternalRouter } from '@daffodil/external-router';

export const appConfig: ApplicationConfig = {
	providers: [
		provideRouter(routes),
		provideClientHydration(),
		provideExternalRouter(),
	],
};
export const routes: Routes = [
	{
		path: '',
		pathMatch: 'full',
		component: HomeComponent,
	},
	{
		path: '**',
		component: TestComponent,
		canMatch: [daffExternalMatcherTypeGuard('TEST_TYPE')],
	},
	{
		path: '**',
		component: OtherTypeComponent,
		canMatch: [daffExternalMatcherTypeGuard('OTHER_TYPE')],
	},
];

Which is much more syntactically direct. It also fixes the original issue as the debug log looks like:

Navigate (id:1)
Navigated (id:1)

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Users should:

  1. Remove their configuration like:
DaffExternalRouterModule.forRoot({
      failedResolutionPath: 'error',
      notFoundResolutionPath: 'not-found',
    }, [
      {
        type: 'CATEGORY',
        insertionStrategy: daffInsertDataPathStrategy,
        route: {},
      },
      {
        type: 'PRODUCT',
        insertionStrategy: daffInsertDataPathStrategy,
        route: {},
      },
    ]),

in favor of

provideExternalRouter({ failedResolutionPath: 'error' }),
  1. Update Route configuration from:
{
        matcher: daffDataPathUrlMatcher,
        data: {
          daffExternalRouteType: 'PRODUCT',
        },
        component: ProductComponent
},

to:

{
    path: '**',
    component: OtherTypeComponent,
    canMatch: [daffExternalMatcherTypeGuard('PRODUCT')],
},

Other information

@damienwebdev damienwebdev changed the title tmp feat!(external-router): rework Jul 11, 2024
@damienwebdev damienwebdev force-pushed the fix_external_router branch 2 times, most recently from c6b41dc to 23f51f0 Compare October 18, 2024 14:30
@damienwebdev damienwebdev changed the title feat!(external-router): rework feat(external-router)!: rework to use canMatchFn Oct 18, 2024
@damienwebdev damienwebdev marked this pull request as ready for review October 18, 2024 14:31
@damienwebdev damienwebdev requested a review from a team as a code owner October 18, 2024 14:31
@damienwebdev damienwebdev requested review from a team as code owners October 18, 2024 17:24
@damienwebdev damienwebdev added the package: external-router @daffodil/external-router label Oct 18, 2024
BREAKING CHANGE: This is a substantial overhaul to the `@daffodil/external-router`. The most important change is the removal of `DaffExternalRouterExistenceGuard` and supporting services in favor of `daffExternalMatcherTypeGuard`. See the docs for exactly how to change your `Routes`.

Additionally, all tokens have been documented along with new guides for the drivers.

Finally, `daffSeoData` is also available on matched routes. This can be used for adjusting titles and schema data.
Copy link
Member

@griest024 griest024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this! So much better

libs/external-router/src/provide-external-router.ts Outdated Show resolved Hide resolved
libs/external-router/src/provide-external-router.ts Outdated Show resolved Hide resolved
libs/external-router/README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: external-router @daffodil/external-router
Projects
None yet
2 participants