Skip to content

Commit 060bd60

Browse files
committed
fix(platform): recognize dynamic redirects in route diagnostics
The dev-time route idiom analyzer only treated a plain string or an expression-free template literal as a redirect, so a page whose `routeMeta.redirectTo` is a function, an identifier or an interpolated template was reported as `missing-default-export` even though the router builds a redirect route for it. It also skipped a `routeMeta` wrapped in `satisfies`, `as`, a type assertion or parentheses. A `redirectTo` property now counts as a redirect unless its value is `null` or `undefined`, which mirrors the runtime. The analyzer unwraps the wrapper expressions before reading the object, keeps the relative-target check for the static part of the target, and reports a component export beside a function redirect as it does beside a string. Closes #2709
1 parent 6443b97 commit 060bd60

2 files changed

Lines changed: 207 additions & 17 deletions

File tree

‎packages/platform/src/lib/route-idiom-diagnostics.spec.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,159 @@ describe('route-idiom-diagnostics', () => {
8181
);
8282
});
8383

84+
it('does not warn about missing default exports for function redirects', () => {
85+
const diagnostics = analyzeAnalogRouteFile({
86+
filename: '/workspace/src/app/pages/users/[userId]/index.page.ts',
87+
code: [
88+
'export const routeMeta = {',
89+
" redirectTo: ({ params }) => `/users/${params['userId']}/profile`,",
90+
" pathMatch: 'full',",
91+
'};',
92+
].join('\n'),
93+
});
94+
95+
expect(diagnostics.map((diagnostic) => diagnostic.code)).not.toContain(
96+
'missing-default-export',
97+
);
98+
});
99+
100+
it('does not warn about missing default exports for identifier redirects', () => {
101+
const diagnostics = analyzeAnalogRouteFile({
102+
filename: '/workspace/src/app/pages/index.page.ts',
103+
code: [
104+
"const redirect = () => '/';",
105+
'',
106+
'export const routeMeta = {',
107+
' redirectTo: redirect,',
108+
" pathMatch: 'full',",
109+
'};',
110+
].join('\n'),
111+
});
112+
113+
expect(diagnostics.map((diagnostic) => diagnostic.code)).not.toContain(
114+
'missing-default-export',
115+
);
116+
});
117+
118+
it('treats an absolute template redirect as a redirect-only route', () => {
119+
const diagnostics = analyzeAnalogRouteFile({
120+
filename: '/workspace/src/app/pages/index.page.ts',
121+
code: [
122+
"const id = 'me';",
123+
'',
124+
'export const routeMeta = {',
125+
' redirectTo: `/users/${id}`,',
126+
" pathMatch: 'full',",
127+
'};',
128+
].join('\n'),
129+
});
130+
131+
const codes = diagnostics.map((diagnostic) => diagnostic.code);
132+
133+
expect(codes).not.toContain('missing-default-export');
134+
expect(codes).not.toContain('relative-redirect');
135+
});
136+
137+
it('warns on relative template redirect targets', () => {
138+
const diagnostics = analyzeAnalogRouteFile({
139+
filename: '/workspace/src/app/pages/cities/index.page.ts',
140+
code: [
141+
"const id = 'new-york';",
142+
'',
143+
'export const routeMeta = {',
144+
' redirectTo: `cities/${id}`,',
145+
" pathMatch: 'full',",
146+
'};',
147+
].join('\n'),
148+
});
149+
150+
expect(diagnostics.map((diagnostic) => diagnostic.code)).toContain(
151+
'relative-redirect',
152+
);
153+
});
154+
155+
it('does not warn on template redirect targets without a static prefix', () => {
156+
const diagnostics = analyzeAnalogRouteFile({
157+
filename: '/workspace/src/app/pages/index.page.ts',
158+
code: [
159+
"const prefix = '/account';",
160+
'',
161+
'export const routeMeta = {',
162+
' redirectTo: `${prefix}/settings`,',
163+
" pathMatch: 'full',",
164+
'};',
165+
].join('\n'),
166+
});
167+
168+
const codes = diagnostics.map((diagnostic) => diagnostic.code);
169+
170+
expect(codes).not.toContain('missing-default-export');
171+
expect(codes).not.toContain('relative-redirect');
172+
});
173+
174+
it.each([
175+
['satisfies', 'export const routeMeta = {', '} satisfies RouteMeta;'],
176+
['as', 'export const routeMeta = {', '} as RouteMeta;'],
177+
['a type assertion', 'export const routeMeta = <RouteMeta>{', '};'],
178+
['parentheses', 'export const routeMeta = ({', '});'],
179+
])(
180+
'does not warn about missing default exports for valid redirect-only routes wrapped in %s',
181+
(_wrapper, open, close) => {
182+
const diagnostics = analyzeAnalogRouteFile({
183+
filename: '/workspace/src/app/pages/index.page.ts',
184+
code: [
185+
"import type { RouteMeta } from '@analogjs/router';",
186+
'',
187+
open,
188+
" redirectTo: '/home',",
189+
" pathMatch: 'full',",
190+
close,
191+
].join('\n'),
192+
});
193+
194+
expect(diagnostics.map((diagnostic) => diagnostic.code)).not.toContain(
195+
'missing-default-export',
196+
);
197+
},
198+
);
199+
200+
it.each(['null', 'undefined', 'null as any'])(
201+
'warns about missing default exports when redirectTo is %s',
202+
(value) => {
203+
const diagnostics = analyzeAnalogRouteFile({
204+
filename: '/workspace/src/app/pages/index.page.ts',
205+
code: [
206+
'export const routeMeta = {',
207+
` redirectTo: ${value},`,
208+
" pathMatch: 'full',",
209+
'};',
210+
].join('\n'),
211+
});
212+
213+
expect(diagnostics.map((diagnostic) => diagnostic.code)).toContain(
214+
'missing-default-export',
215+
);
216+
},
217+
);
218+
219+
it('warns when function redirect routes also default-export a component', () => {
220+
const diagnostics = analyzeAnalogRouteFile({
221+
filename: '/workspace/src/app/pages/index.page.ts',
222+
code: [
223+
'export const routeMeta = {',
224+
" redirectTo: () => '/home',",
225+
" pathMatch: 'full',",
226+
'};',
227+
'',
228+
'export default class HomeRedirectPage {}',
229+
].join('\n'),
230+
});
231+
232+
expect(diagnostics.map((diagnostic) => diagnostic.code)).toContain(
233+
'redirect-with-component',
234+
);
235+
});
236+
84237
it('warns when a layout-looking route does not reference RouterOutlet', () => {
85238
const diagnostics = analyzeAnalogRouteFile({
86239
filename: '/workspace/src/app/pages/products.page.ts',

‎packages/platform/src/lib/route-idiom-diagnostics.ts‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ export interface AnalyzeAnalogRouteFileOptions {
1818
const PAGE_FILE_RE = /\.page\.ts$/;
1919
const ROUTER_OUTLET_RE = /\bRouterOutlet\b|<router-outlet(?:\s|>)/;
2020

21+
// Valid AST node types: grouping parentheses, as, satisfies, angle-bracket type assertion
22+
const WRAPPER_EXPRESSIONS = new Set([
23+
'ParenthesizedExpression',
24+
'TSAsExpression',
25+
'TSSatisfiesExpression',
26+
'TSTypeAssertion',
27+
]);
28+
2129
export function analyzeAnalogRouteFile(
2230
options: AnalyzeAnalogRouteFileOptions,
2331
): AnalogRouteIdiomDiagnostic[] {
@@ -69,9 +77,8 @@ export function analyzeAnalogRouteFile(
6977
}
7078

7179
if (
72-
routeMetaInfo?.hasRedirect &&
73-
routeMetaInfo.redirectTo &&
74-
!routeMetaInfo.redirectTo.startsWith('/')
80+
routeMetaInfo?.redirectTarget &&
81+
!routeMetaInfo.redirectTarget.startsWith('/')
7582
) {
7683
diagnostics.push({
7784
code: 'relative-redirect',
@@ -158,32 +165,42 @@ function getRouteMetaInfo(
158165
bindingName: string,
159166
): {
160167
hasRedirect: boolean;
161-
redirectTo?: string;
168+
redirectTarget?: string;
162169
} | null {
163-
const routeMetaNode = getExportedBindingInitializer(program, bindingName);
170+
const routeMetaNode = unwrapExpression(
171+
getExportedBindingInitializer(program, bindingName),
172+
);
164173

165174
if (routeMetaNode?.type !== 'ObjectExpression') {
166175
return null;
167176
}
168177

169-
let redirectTo: string | undefined;
178+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
179+
let redirectTo: any;
170180

171181
for (const property of routeMetaNode.properties ?? []) {
172182
if (property?.type !== 'Property') {
173183
continue;
174184
}
175185

176186
const keyName = getPropertyName(property.key);
187+
177188
if (keyName !== 'redirectTo') {
178189
continue;
179190
}
180191

181-
redirectTo = getStringValue(property.value);
192+
redirectTo = property.value;
193+
}
194+
195+
const value = unwrapExpression(redirectTo);
196+
197+
if (!value || isNullish(value)) {
198+
return { hasRedirect: false };
182199
}
183200

184201
return {
185-
hasRedirect: typeof redirectTo === 'string' && redirectTo.length > 0,
186-
redirectTo,
202+
hasRedirect: true,
203+
redirectTarget: getStaticRedirectTarget(value),
187204
};
188205
}
189206

@@ -312,24 +329,44 @@ function getPropertyName(
312329
return undefined;
313330
}
314331

315-
function getStringValue(
332+
function getStaticRedirectTarget(
316333
// eslint-disable-next-line @typescript-eslint/no-explicit-any
317334
node: any,
318335
): string | undefined {
319336
if (
320-
(node?.type === 'Literal' || node?.type === 'StringLiteral') &&
337+
(node.type === 'Literal' || node.type === 'StringLiteral') &&
321338
typeof node.value === 'string'
322339
) {
323340
return node.value;
324341
}
325342

326-
if (
327-
node?.type === 'TemplateLiteral' &&
328-
node.expressions?.length === 0 &&
329-
node.quasis?.length === 1
330-
) {
331-
return node.quasis[0].value.cooked ?? node.quasis[0].value.raw;
343+
if (node.type === 'TemplateLiteral') {
344+
const lead = node.quasis?.[0]?.value.cooked ?? node.quasis?.[0]?.value.raw;
345+
return lead || undefined;
332346
}
333347

334348
return undefined;
335349
}
350+
351+
function isNullish(
352+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
353+
node: any,
354+
): boolean {
355+
return (
356+
(node.type === 'Literal' && node.value === null) ||
357+
(node.type === 'Identifier' && node.name === 'undefined') ||
358+
(node.type === 'UnaryExpression' && node.operator === 'void')
359+
);
360+
}
361+
362+
function unwrapExpression(
363+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
364+
node: any,
365+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
366+
): any | undefined {
367+
while (node && WRAPPER_EXPRESSIONS.has(node.type)) {
368+
node = node.expression;
369+
}
370+
371+
return node;
372+
}

0 commit comments

Comments
 (0)