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

Calling recognize should not affect the transition.from query params for subsequent transitions #336

Merged
merged 3 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions lib/router/route-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
model?(params: Dict<unknown>, transition: Transition): PromiseLike<T> | undefined | T;
deserialize?(params: Dict<unknown>, transition: Transition): T | PromiseLike<T> | undefined;
serialize?(model: T | undefined, params: string[]): Dict<unknown> | undefined;
beforeModel?(transition: Transition): PromiseLike<any> | any;

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type

Check warning on line 29 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
afterModel?(resolvedModel: T | undefined, transition: Transition): PromiseLike<any> | any;

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type

Check warning on line 30 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
setup?(context: T | undefined, transition: Transition): void;
enter?(transition: Transition): void;
exit?(transition?: Transition): void;
Expand All @@ -47,13 +47,13 @@
readonly queryParams: Dict<unknown>;
readonly metadata: unknown;
find(
predicate: (this: any, routeInfo: RouteInfo, i: number) => boolean,

Check warning on line 50 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 50 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 50 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 50 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
thisArg?: any

Check warning on line 51 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 51 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 51 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 51 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
): RouteInfo | undefined;
}

export interface RouteInfoWithAttributes extends RouteInfo {
attributes: any;

Check warning on line 56 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 56 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 56 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 56 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
}

type RouteInfosKey = InternalRouteInfo<Route>;
Expand All @@ -63,23 +63,34 @@
export function toReadOnlyRouteInfo<R extends Route>(
routeInfos: InternalRouteInfo<R>[],
queryParams: Dict<unknown> = {},
includeAttributes = false
options: {
includeAttributes?: boolean;
localizeMapUpdates?: boolean;
} = { includeAttributes: false, localizeMapUpdates: false }
): RouteInfoWithAttributes[] | RouteInfo[] {
const LOCAL_ROUTE_INFOS = new WeakMap<RouteInfosKey, RouteInfo | RouteInfoWithAttributes>();

return routeInfos.map((info, i) => {
let { name, params, paramNames, context, route } = info;
// SAFETY: This should be safe since it is just for use as a key
let key = (info as unknown) as RouteInfosKey;
if (ROUTE_INFOS.has(key) && includeAttributes) {
if (ROUTE_INFOS.has(key) && options.includeAttributes) {
let routeInfo = ROUTE_INFOS.get(key)!;
routeInfo = attachMetadata(route!, routeInfo);
let routeInfoWithAttribute = createRouteInfoWithAttributes(routeInfo, context);
ROUTE_INFOS.set(key, routeInfoWithAttribute);
LOCAL_ROUTE_INFOS.set(key, routeInfo);
if (!options.localizeMapUpdates) {
ROUTE_INFOS.set(key, routeInfoWithAttribute);
}
return routeInfoWithAttribute as RouteInfoWithAttributes;
}

const routeInfosRef = options.localizeMapUpdates ? LOCAL_ROUTE_INFOS : ROUTE_INFOS;

let routeInfo: RouteInfo = {
find(
predicate: (this: any, routeInfo: RouteInfo, i: number, arr?: RouteInfo[]) => boolean,

Check warning on line 92 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 92 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 92 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 92 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
thisArg: any

Check warning on line 93 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 93 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 93 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 93 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
) {
let publicInfo;
let arr: RouteInfo[] = [];
Expand All @@ -87,13 +98,13 @@
if (predicate.length === 3) {
arr = routeInfos.map(
// SAFETY: This should be safe since it is just for use as a key
(info) => ROUTE_INFOS.get((info as unknown) as RouteInfosKey)!
(info) => routeInfosRef.get((info as unknown) as RouteInfosKey)!
);
}

for (let i = 0; routeInfos.length > i; i++) {
// SAFETY: This should be safe since it is just for use as a key
publicInfo = ROUTE_INFOS.get((routeInfos[i] as unknown) as RouteInfosKey)!;
publicInfo = routeInfosRef.get((routeInfos[i] as unknown) as RouteInfosKey)!;
if (predicate.call(thisArg, publicInfo, i, arr)) {
return publicInfo;
}
Expand Down Expand Up @@ -122,7 +133,7 @@
}

// SAFETY: This should be safe since it is just for use as a key
return ROUTE_INFOS.get((parent as unknown) as RouteInfosKey)!;
return routeInfosRef.get((parent as unknown) as RouteInfosKey)!;
},

get child() {
Expand All @@ -133,7 +144,7 @@
}

// SAFETY: This should be safe since it is just for use as a key
return ROUTE_INFOS.get((child as unknown) as RouteInfosKey)!;
return routeInfosRef.get((child as unknown) as RouteInfosKey)!;
},

get localName() {
Expand All @@ -150,12 +161,17 @@
},
};

if (includeAttributes) {
if (options.includeAttributes) {
routeInfo = createRouteInfoWithAttributes(routeInfo, context);
}

// SAFETY: This should be safe since it is just for use as a key
ROUTE_INFOS.set((info as unknown) as RouteInfosKey, routeInfo);
LOCAL_ROUTE_INFOS.set((info as unknown) as RouteInfosKey, routeInfo);

if (!options.localizeMapUpdates) {
// SAFETY: This should be safe since it is just for use as a key
ROUTE_INFOS.set((info as unknown) as RouteInfosKey, routeInfo);
}

return routeInfo;
});
Expand All @@ -163,7 +179,7 @@

function createRouteInfoWithAttributes(
routeInfo: RouteInfo,
context: any

Check warning on line 182 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (12)

Unexpected any. Specify a different type

Check warning on line 182 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (14)

Unexpected any. Specify a different type

Check warning on line 182 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (16)

Unexpected any. Specify a different type

Check warning on line 182 in lib/router/route-info.ts

View workflow job for this annotation

GitHub Actions / Tests (17)

Unexpected any. Specify a different type
): RouteInfoWithAttributes {
let attributes = {
get attributes() {
Expand Down
21 changes: 13 additions & 8 deletions lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ export default abstract class Router<R extends Route> {
return newState;
}

let readonlyInfos = toReadOnlyRouteInfo(newState.routeInfos, newState.queryParams);
let readonlyInfos = toReadOnlyRouteInfo(newState.routeInfos, newState.queryParams, {
includeAttributes: false,
localizeMapUpdates: true,
});
return readonlyInfos[readonlyInfos.length - 1] as RouteInfo;
}

Expand All @@ -188,7 +191,10 @@ export default abstract class Router<R extends Route> {
let routeInfosWithAttributes = toReadOnlyRouteInfo(
newState!.routeInfos,
newTransition[QUERY_PARAMS_SYMBOL],
true
{
includeAttributes: true,
localizeMapUpdates: false,
}
) as RouteInfoWithAttributes[];
return routeInfosWithAttributes[routeInfosWithAttributes.length - 1];
});
Expand Down Expand Up @@ -773,11 +779,10 @@ export default abstract class Router<R extends Route> {

private fromInfos(newTransition: OpaqueTransition, oldRouteInfos: InternalRouteInfo<R>[]) {
if (newTransition !== undefined && oldRouteInfos.length > 0) {
let fromInfos = toReadOnlyRouteInfo(
oldRouteInfos,
Object.assign({}, this._lastQueryParams),
true
) as RouteInfoWithAttributes[];
let fromInfos = toReadOnlyRouteInfo(oldRouteInfos, Object.assign({}, this._lastQueryParams), {
includeAttributes: true,
localizeMapUpdates: false,
}) as RouteInfoWithAttributes[];
newTransition!.from = fromInfos[fromInfos.length - 1] || null;
}
}
Expand All @@ -791,7 +796,7 @@ export default abstract class Router<R extends Route> {
let toInfos = toReadOnlyRouteInfo(
newRouteInfos,
Object.assign({}, newTransition[QUERY_PARAMS_SYMBOL]),
includeAttributes
{ includeAttributes, localizeMapUpdates: false }
);
newTransition!.to = toInfos[toInfos.length - 1] || null;
}
Expand Down
61 changes: 61 additions & 0 deletions tests/router_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,67 @@ scenarios.forEach(function (scenario) {
});
});

test('calling recognize should not affect the transition.from query params for subsequent transitions', function (assert) {
assert.expect(12);
map(assert, function (match) {
match('/').to('index');
match('/search').to('search');
});

routes = {
search: createHandler('search'),
};

let firstParam = false;
let secondParam = false;

router.routeWillChange = (transition: Transition) => {
if (secondParam) {
assert.deepEqual(transition.to!.queryParams, { term: 'c' }, 'going to next page with qps');
assert.deepEqual(
isPresent(transition.from) && transition.from!.queryParams,
{ term: 'b' },
'has previous qps'
);
} else if (firstParam) {
assert.deepEqual(transition.to!.queryParams, { term: 'b' }, 'going to page with qps');
assert.deepEqual(
isPresent(transition.from) && transition.from!.queryParams,
{},
'from never has qps'
);
} else {
assert.equal(transition.from, null);
assert.deepEqual(transition.to!.queryParams, {});
}
};

router.routeDidChange = (transition: Transition) => {
if (secondParam) {
assert.deepEqual(transition.to!.queryParams, { term: 'c' });
assert.deepEqual(isPresent(transition.from) && transition.from!.queryParams, { term: 'b' });
} else if (firstParam) {
assert.deepEqual(transition.to!.queryParams, { term: 'b' });
assert.deepEqual(isPresent(transition.from) && transition.from!.queryParams, {});
} else {
assert.equal(transition.from, null);
assert.deepEqual(transition.to!.queryParams, {});
}
};

router
.transitionTo('/')
.then(() => {
firstParam = true;
return router.transitionTo('search', { queryParams: { term: 'b' } });
})
.then(() => {
secondParam = true;
router.recognize('/search?wat=foo');
return router.transitionTo({ queryParams: { term: 'c' } });
});
});

test('redirects route events', function (assert) {
assert.expect(19);
map(assert, function (match) {
Expand Down
Loading