Skip to content

Commit

Permalink
fix(router-lite): excluded redirectTo from nav-model (#1816)
Browse files Browse the repository at this point in the history 
* fix(router-lite): excluded redirectTo from nav-model

#1815

* chore: updated chromedriver

* chore: cleanup
  • Loading branch information
@Sayan751
Sayan751 committed Oct 6, 2023
1 parent de79aea commit 085a491
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 42 deletions.
16 changes: 8 additions & 8 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"@typescript-eslint/eslint-plugin": "5.51.0",
"@typescript-eslint/parser": "5.51.0",
"chalk": "4.1.2",
"chromedriver": "114.0.3",
"chromedriver": "117.0.3",
"codecov": "^3.8.3",
"concurrently": "7.6.0",
"cross-env": "7.0.3",
Expand Down
25 changes: 12 additions & 13 deletions packages/__tests__/src/router-lite/smoke-tests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3847,7 +3847,6 @@ describe('router-lite/smoke-tests.spec.ts', function () {

function getNavBarCe(
hasAsyncRouteConfig: boolean = false,
includeRedirection: boolean = false,
) {
@valueConverter('firstNonEmpty')
class FirstNonEmpty {
Expand All @@ -3862,7 +3861,7 @@ describe('router-lite/smoke-tests.spec.ts', function () {
name: 'nav-bar',
template: `<nav if.bind="navModel">
<ul>
<li repeat.for="route of navModel.routes${includeRedirection ? '' : '.filter(r => !r.redirectTo)'}"><a href.bind="route.path | firstNonEmpty" active.class="route.isActive">\${route.title}</a></li>
<li repeat.for="route of navModel.routes"><a href.bind="route.path | firstNonEmpty" active.class="route.isActive">\${route.title}</a></li>
</ul>
</nav><template else>no nav model</template>`,
dependencies: [FirstNonEmpty]
Expand Down Expand Up @@ -4430,7 +4429,7 @@ describe('router-lite/smoke-tests.spec.ts', function () {
await au.stop(true);
});

it('with redirection - path with redirection is shown', async function () {
it('with redirection - path with redirection is not shown', async function () {
@customElement({ name: 'ce-c11', template: 'c11' })
class C11 { }
@customElement({ name: 'ce-c12', template: 'c12' })
Expand Down Expand Up @@ -4476,7 +4475,7 @@ describe('router-lite/smoke-tests.spec.ts', function () {
const ctx = TestContext.create();
const { container } = ctx;

const navBarCe = getNavBarCe(false, true);
const navBarCe = getNavBarCe(false);
container.register(
StandardConfiguration,
TestRouterConfiguration.for(LogLevel.warn),
Expand All @@ -4496,35 +4495,35 @@ describe('router-lite/smoke-tests.spec.ts', function () {
await queue.yield();
type NavBar = InstanceType<typeof navBarCe>;
const rootNavbar = CustomElement.for<NavBar>(host.querySelector('nav-bar')).viewModel;
rootNavbar.assert([{ href: '', text: '', active: true }, { href: 'p1', text: 'P1', active: true }, { href: 'p2', text: 'P2', active: false }], 'start root');
rootNavbar.assert([{ href: 'p1', text: 'P1', active: true }, { href: 'p2', text: 'P2', active: false }], 'start root');
let childNavBar = CustomElement.for<NavBar>(host.querySelector('ce-p1>nav-bar')).viewModel;
childNavBar.assert([{ href: '', text: '', active: true }, { href: 'c11', text: 'C11', active: true }, { href: 'c12', text: 'C12', active: false }], 'start child navbar');
childNavBar.assert([{ href: 'c11', text: 'C11', active: true }, { href: 'c12', text: 'C12', active: false }], 'start child navbar');

// Round#1
await router.load('p2');
await queue.yield();
rootNavbar.assert([{ href: '', text: '', active: false }, { href: 'p1', text: 'P1', active: false }, { href: 'p2', text: 'P2', active: true }], 'round#1 root');
rootNavbar.assert([{ href: 'p1', text: 'P1', active: false }, { href: 'p2', text: 'P2', active: true }], 'round#1 root');
childNavBar = CustomElement.for<NavBar>(host.querySelector('ce-p2>nav-bar')).viewModel;
childNavBar.assert([{ href: '', text: '', active: true }, { href: 'c21', text: 'C21', active: false }, { href: 'c22', text: 'C22', active: true }], 'round#1 child navbar');
childNavBar.assert([{ href: 'c21', text: 'C21', active: false }, { href: 'c22', text: 'C22', active: true }], 'round#1 child navbar');

// Round#2
await router.load('p1/c12');
await queue.yield();
rootNavbar.assert([{ href: '', text: '', active: false }, { href: 'p1', text: 'P1', active: true }, { href: 'p2', text: 'P2', active: false }], 'round#2 root');
rootNavbar.assert([{ href: 'p1', text: 'P1', active: true }, { href: 'p2', text: 'P2', active: false }], 'round#2 root');
childNavBar = CustomElement.for<NavBar>(host.querySelector('ce-p1>nav-bar')).viewModel;
childNavBar.assert([{ href: '', text: '', active: false }, { href: 'c11', text: 'C11', active: false }, { href: 'c12', text: 'C12', active: true }], 'round#2 navbar');
childNavBar.assert([{ href: 'c11', text: 'C11', active: false }, { href: 'c12', text: 'C12', active: true }], 'round#2 navbar');

// Round#3
await router.load('p2/c21');
await queue.yield();
rootNavbar.assert([{ href: '', text: '', active: false }, { href: 'p1', text: 'P1', active: false }, { href: 'p2', text: 'P2', active: true }], 'round#3 root');
rootNavbar.assert([{ href: 'p1', text: 'P1', active: false }, { href: 'p2', text: 'P2', active: true }], 'round#3 root');
childNavBar = CustomElement.for<NavBar>(host.querySelector('ce-p2>nav-bar')).viewModel;
childNavBar.assert([{ href: '', text: '', active: false }, { href: 'c21', text: 'C21', active: true }, { href: 'c22', text: 'C22', active: false }], 'round#3 navbar');
childNavBar.assert([{ href: 'c21', text: 'C21', active: true }, { href: 'c22', text: 'C22', active: false }], 'round#3 navbar');

// Round#4 - nav:false, but routeable
await router.load('p3');
await queue.yield();
rootNavbar.assert([{ href: '', text: '', active: false }, { href: 'p1', text: 'P1', active: false }, { href: 'p2', text: 'P2', active: false }], 'round#4 root');
rootNavbar.assert([{ href: 'p1', text: 'P1', active: false }, { href: 'p2', text: 'P2', active: false }], 'round#4 root');
assert.notEqual(host.querySelector('ce-p3'), null);

await au.stop(true);
Expand Down
8 changes: 3 additions & 5 deletions packages/router-lite/src/route-context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ class NavigationModel implements INavigationModel {
public _addRoute(route: RouteConfig | Promise<RouteConfig>): void {
const routes = this.routes;
if (!(route instanceof Promise)) {
if (route.nav ?? false) {
if ((route.nav ?? false) && route.redirectTo === null) {
routes.push(NavigationRoute._create(route));
}
return;
Expand All @@ -722,7 +722,7 @@ class NavigationModel implements INavigationModel {
let promise: void | Promise<void> = void 0;
promise = this._promise = onResolve(this._promise, () =>
onResolve(route, rdConfig => {
if (rdConfig.nav) {
if (rdConfig.nav && rdConfig.redirectTo === null) {
routes[index] = NavigationRoute._create(rdConfig);
} else {
routes.splice(index, 1);
Expand All @@ -749,7 +749,6 @@ class NavigationRoute implements INavigationRoute {
private constructor(
public readonly id: string | null,
public readonly path: string[],
public readonly redirectTo: string | null,
public readonly title: string | ((node: RouteNode) => string | null) | null,
public readonly data: Record<string, unknown>,
) { }
Expand All @@ -759,7 +758,6 @@ class NavigationRoute implements INavigationRoute {
return new NavigationRoute(
rdConfig.id,
ensureArrayOfStrings(rdConfig.path ?? emptyArray),
rdConfig.redirectTo,
rdConfig.title,
rdConfig.data,
);
Expand Down Expand Up @@ -791,6 +789,6 @@ class NavigationRoute implements INavigationRoute {
);
});
}
this._isActive = trees.some(vit => router.routeTree.contains(vit, { matchEndpoint: true, matchOriginalInstruction: this.redirectTo !== null }));
this._isActive = trees.some(vit => router.routeTree.contains(vit, true));
}
}
19 changes: 5 additions & 14 deletions packages/router-lite/src/route-tree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,6 @@ export interface IRouteNode {
residue?: ViewportInstruction[];
}

export interface RouteNodeMatchOptions {
/** Endpoints will be matched instead of instruction */
matchEndpoint: boolean;
/** Original instruction will be matched */
matchOriginalInstruction: boolean;
}

export class RouteNode implements IRouteNode {
/** @internal */ public _tree!: RouteTree;
/** @internal */ public _version: number = 1;
Expand Down Expand Up @@ -161,10 +154,8 @@ export class RouteNode implements IRouteNode {
);
}

public contains(instructions: ViewportInstructionTree, options: Partial<RouteNodeMatchOptions>): boolean {
public contains(instructions: ViewportInstructionTree, matchEndpoint: boolean = false): boolean {
if (this.context === instructions.options.context) {
const matchEndpoint = options.matchEndpoint ?? false;
const matchOriginalInstruction = options.matchOriginalInstruction ?? false;
const nodeChildren = this.children;
const instructionChildren = instructions.children;
for (let i = 0, ii = nodeChildren.length; i < ii; ++i) {
Expand All @@ -173,7 +164,7 @@ export class RouteNode implements IRouteNode {
const instructionEndpoint = matchEndpoint ? instructionChild.recognizedRoute?.route.endpoint : null;
const nodeChild = nodeChildren[i + j] ?? null;
const instruction = nodeChild !== null
? !matchOriginalInstruction && nodeChild.isInstructionsFinalized ? nodeChild.instruction : nodeChild._originalInstruction
? nodeChild.isInstructionsFinalized ? nodeChild.instruction : nodeChild._originalInstruction
: null;
const childEndpoint = instruction?.recognizedRoute?.route.endpoint;
if (i + j < ii
Expand All @@ -193,7 +184,7 @@ export class RouteNode implements IRouteNode {
}

return this.children.some(function (x) {
return x.contains(instructions, options);
return x.contains(instructions, matchEndpoint);
});
}

Expand Down Expand Up @@ -314,8 +305,8 @@ export class RouteTree {
public root: RouteNode,
) { }

public contains(instructions: ViewportInstructionTree, options: Partial<RouteNodeMatchOptions>): boolean {
return this.root.contains(instructions, options);
public contains(instructions: ViewportInstructionTree, matchEndpoint: boolean = false): boolean {
return this.root.contains(instructions, matchEndpoint);
}

/** @internal */
Expand Down
2 changes: 1 addition & 1 deletion packages/router-lite/src/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ export class Router {

if (__DEV__) trace(this._logger, Events.rtrIsActive, instructions, ctx);

return this.routeTree.contains(instructions, { matchEndpoint: false });
return this.routeTree.contains(instructions, false);
}

private readonly _vpaLookup: ViewportAgentLookup = new Map();
Expand Down

0 comments on commit 085a491

Please sign in to comment.