Skip to content

Commit f93a892

Browse files
committed
Serve deeplinks from /_ instead of the deeplink literal
Escape the underscore in the route filename so it stays a literal URL segment: a flat-route segment beginning with `_` is a pathless layout, so `_.$` would have mounted the route at `/*`. `createRoutePath` skips a segment only when its cooked and raw spellings both start with `_`, and the raw spelling of `[_]` does not, so `[_].$` serves `/_/*`. The prefix comparison no longer folds case. It existed because React Router matches routes case-insensitively and `/Deeplink/apikeys` really did reach the loader, but `_` has no case, so the fold is dead. Page-name folding stays, since `/_/APIKeys` still has a route to agree with. Drops `/deeplink/*` rather than keeping it as an alias; the route has not shipped, so nothing links to it yet.
1 parent 6b2e0f9 commit f93a892

4 files changed

Lines changed: 64 additions & 47 deletions

File tree

.server-changes/deeplink-routes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ area: webapp
33
type: feature
44
---
55

6-
Short links like /deeplink/apikeys now take you straight to that page in your current project and environment, so you no longer need the full URL with your org, project and environment in it.
6+
Short links like /_/apikeys now take you straight to that page in your current project and environment, so you no longer need the full URL with your org, project and environment in it.
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ import {
1212
} from "~/utils/pathBuilder";
1313

1414
/**
15-
* Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to
15+
* Stable links that don't name an org, project or environment: /_/apikeys redirects to
1616
* /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the pages in
1717
* ENV_PAGE_TARGETS are followed, so an unrecognised path can never become the redirect target —
1818
* it lands on the resolved environment instead.
19+
*
20+
* The filename escapes the underscore for a reason — see DEEPLINK_PATH_PREFIX.
1921
*/
2022
export const loader = async ({ request }: LoaderFunctionArgs) => {
2123
const user = await requireUser(request);

apps/webapp/app/utils/deeplinkPages.test.ts

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe("deeplink targets", () => {
105105
});
106106

107107
it("every deep path lands on a real route", () => {
108-
// The invariant a bare segment list could not express: /deeplink/waitpoints/{id} has to reach
108+
// The invariant a bare segment list could not express: /_/waitpoints/{id} has to reach
109109
// waitpoints/tokens/{id}, not waitpoints/{id}. Driven off the real child routes rather than one
110110
// synthetic segment, so names whose children are all literal (settings/general) count too.
111111
const broken: string[] = [];
@@ -272,54 +272,60 @@ describe("resolveDeeplinkPage", () => {
272272

273273
describe("deeplinkSuffix", () => {
274274
it("strips the route's own prefix", () => {
275-
expect(deeplinkSuffix("/deeplink/tasks")).toBe("tasks");
276-
expect(deeplinkSuffix("/deeplink/runs/run_123")).toBe("runs/run_123");
275+
expect(deeplinkSuffix("/_/tasks")).toBe("tasks");
276+
expect(deeplinkSuffix("/_/runs/run_123")).toBe("runs/run_123");
277277
});
278278

279279
it("keeps an escaped slash intact, unlike the decoded splat param", () => {
280-
expect(deeplinkSuffix("/deeplink/tasks/standard/group%2Fmy-task")).toBe(
280+
expect(deeplinkSuffix("/_/tasks/standard/group%2Fmy-task")).toBe(
281281
"tasks/standard/group%2Fmy-task"
282282
);
283283
});
284284

285-
it("strips the prefix whatever its case, and only the prefix", () => {
286-
expect(deeplinkSuffix("/Deeplink/apikeys")).toBe("apikeys");
287-
expect(deeplinkSuffix("/DEEPLINK/runs/run_ABC123")).toBe("runs/run_ABC123");
288-
// The remainder comes back as it was written, capitals and all.
289-
expect(deeplinkSuffix("/DeepLink/tasks/standard/My-Task")).toBe("tasks/standard/My-Task");
290-
expect(deeplinkSuffix("/Deeplink")).toBe("");
291-
expect(deeplinkSuffix("/Deeplink/")).toBe("");
285+
it("strips only the prefix, leaving the remainder's case alone", () => {
286+
// The prefix has no case to fold — `_` is the same character either way — so unlike the page
287+
// name there is no case-insensitive comparison here. What still matters is that the remainder
288+
// comes back exactly as written, capitals and all, because ids are case-sensitive.
289+
expect(deeplinkSuffix("/_/runs/run_ABC123")).toBe("runs/run_ABC123");
290+
expect(deeplinkSuffix("/_/tasks/standard/My-Task")).toBe("tasks/standard/My-Task");
292291
});
293292

294-
it("folds case because the route it is mounted on does", () => {
295-
// The assertion the test above rests on: React Router compiles a route path with the `i` flag
296-
// unless it opts into `caseSensitive`, so a capitalised prefix really does reach this loader
297-
// instead of 404ing before it. If that ever changed, the folding would be dead weight.
293+
it("is mounted where the route filename says it is", () => {
294+
// `[_].$` is an escaped literal, not a pathless layout: Remix's `createRoutePath` drops a
295+
// segment only when the cooked and the raw spelling both start with `_`, and the raw spelling
296+
// is `[_]`. A plain `_.$` would compile to `/*` and swallow the site, so this pins the prefix
297+
// the loader strips to the URL the router actually serves.
298298
const route = `${DEEPLINK_PATH_PREFIX}/*`;
299-
expect(matchPath(route, "/deeplink/apikeys")?.params["*"]).toBe("apikeys");
300-
expect(matchPath(route, "/Deeplink/apikeys")?.params["*"]).toBe("apikeys");
301-
// And the splat keeps the case it was given, which is why only the first segment is folded.
302-
expect(matchPath(route, "/DEEPLINK/APIKeys")?.params["*"]).toBe("APIKeys");
299+
expect(route).toBe("/_/*");
300+
expect(matchPath(route, "/_/apikeys")?.params["*"]).toBe("apikeys");
301+
expect(matchPath(route, "/_/runs/run_123")?.params["*"]).toBe("runs/run_123");
302+
// The splat keeps the case it was given, which is why the loader folds only the page name.
303+
expect(matchPath(route, "/_/APIKeys")?.params["*"]).toBe("APIKeys");
304+
// And it is a literal segment, so it matches nothing else.
305+
expect(matchPath(route, "/deeplink/apikeys")).toBeNull();
306+
expect(matchPath(route, "/apikeys")).toBeNull();
303307
});
304308

305309
it("treats a bare prefix, a trailing slash and anything outside it as no suffix", () => {
306-
expect(deeplinkSuffix("/deeplink")).toBe("");
307-
expect(deeplinkSuffix("/deeplink/")).toBe("");
310+
expect(deeplinkSuffix("/_")).toBe("");
311+
expect(deeplinkSuffix("/_/")).toBe("");
308312
// What `new URL` leaves behind once it has normalised and resolved `%2e%2e` itself.
309313
expect(deeplinkSuffix("/etc")).toBe("");
314+
// A prefix that merely starts with the same character is not this route.
315+
expect(deeplinkSuffix("/_app/orgs")).toBe("");
310316
});
311317

312318
it("matches what the URL parser actually produces", () => {
313319
// The behaviour above is only correct if `new URL` really does keep %2F and really does
314320
// resolve %2e%2e, so assert that rather than assuming it.
315-
const encodedSlash = new URL("http://x/deeplink/tasks/standard/group%2Fmy-task");
321+
const encodedSlash = new URL("http://x/_/tasks/standard/group%2Fmy-task");
316322
expect(deeplinkSuffix(encodedSlash.pathname)).toBe("tasks/standard/group%2Fmy-task");
317323
expect(resolveDeeplinkPage(deeplinkSuffix(encodedSlash.pathname))).toBe(
318324
"tasks/standard/group%2Fmy-task"
319325
);
320326

321327
// `%2e%2e` is normalised to `..` and resolved by the parser, leaving the prefix behind.
322-
const traversal = new URL("http://x/deeplink/runs/%2e%2e/%2e%2e/etc");
328+
const traversal = new URL("http://x/_/runs/%2e%2e/%2e%2e/etc");
323329
expect(traversal.pathname).toBe("/etc");
324330
expect(resolveDeeplinkPage(deeplinkSuffix(traversal.pathname))).toBeUndefined();
325331
});

apps/webapp/app/utils/deeplinkPages.ts

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
2-
* Where each /deeplink/<name> goes, relative to the resolved environment.
2+
* Where each /_/<name> goes, relative to the resolved environment.
33
*
44
* Two pieces, because a name's own page and the things underneath it are not always in the same
5-
* place. `landing` is used for a bare `/deeplink/<name>`; `prefix` is what deeper segments hang off.
5+
* place. `landing` is used for a bare `/_/<name>`; `prefix` is what deeper segments hang off.
66
* They differ only where a segment is not a page in its own right:
77
*
88
* - `tasks` has no bare route, and the task list is the environment root — but task detail pages do
@@ -16,9 +16,9 @@
1616
* route files and fails if a page is missing or a target stops resolving.
1717
*/
1818
export type DeeplinkTarget = {
19-
/** Path for a bare `/deeplink/<name>`. "" is the environment root. */
19+
/** Path for a bare `/_/<name>`. "" is the environment root. */
2020
landing: string;
21-
/** Deeper segments are appended to this: `/deeplink/<name>/a/b` -> `<prefix>/a/b`. */
21+
/** Deeper segments are appended to this: `/_/<name>/a/b` -> `<prefix>/a/b`. */
2222
prefix: string;
2323
};
2424

@@ -57,27 +57,35 @@ export const ENV_PAGE_TARGETS: ReadonlyMap<string, DeeplinkTarget> = new Map([
5757
["waitpoints", { landing: "waitpoints/tokens", prefix: "waitpoints/tokens" }],
5858
]);
5959

60-
/** Where this route is mounted. Matches the `deeplink.$` route filename. */
61-
export const DEEPLINK_PATH_PREFIX = "/deeplink";
60+
/**
61+
* Where this route is mounted. Matches the `[_].$` route filename.
62+
*
63+
* The brackets are Remix's escape, and they are load-bearing rather than decorative: a flat-route
64+
* segment that starts with `_` is a pathless layout and contributes nothing to the URL, so a plain
65+
* `_.$` would mount this at `/*` and swallow the whole site. Escaping the underscore makes it a
66+
* literal segment — `createRoutePath` skips a segment only when the cooked *and* the raw spelling
67+
* both start with `_`, and the raw spelling here is `[_]`, so `[_].$` really does serve `/_/*`.
68+
*/
69+
export const DEEPLINK_PATH_PREFIX = "/_";
6270

6371
/**
64-
* The still-encoded suffix after /deeplink, taken from the request's pathname rather than the
65-
* splat param. React Router decodes the splat, which turns an id containing an escaped slash
72+
* The still-encoded suffix after /_, taken from the request's pathname rather than the splat param.
73+
* React Router decodes the splat, which turns an id containing an escaped slash
6674
* (`group%2Fmy-task`, as the dashboard's own link builder writes it) into two segments that match
6775
* no route. The pathname keeps `%2F` intact.
6876
*
6977
* Returns "" for anything that is not under the prefix. That includes a pathname the URL parser has
7078
* already rewritten: it normalises `%2e%2e` to `..` and resolves it, so a traversal attempt can
7179
* leave the prefix entirely before this ever sees it.
7280
*
73-
* The prefix is matched case-insensitively because React Router's route matching is: it compiles
74-
* every path with the `i` flag unless the route opts into `caseSensitive`, so `/Deeplink/apikeys`
75-
* reaches this loader too. Only the prefix is folded — the remainder is returned as it was written,
76-
* since the ids after the first segment are case-sensitive.
81+
* The comparison is exact, unlike the page name's. React Router still matches the route
82+
* case-insensitively, but `_` has no case for it to differ in, so there is nothing to fold.
83+
* The remainder is returned as it was written, since the ids after the first segment are
84+
* case-sensitive.
7785
*/
7886
export function deeplinkSuffix(pathname: string): string {
7987
const withSlash = `${DEEPLINK_PATH_PREFIX}/`;
80-
if (!pathname.toLowerCase().startsWith(withSlash)) return "";
88+
if (!pathname.startsWith(withSlash)) return "";
8189

8290
return pathname.slice(withSlash.length);
8391
}
@@ -103,20 +111,21 @@ function isUsableSegment(segment: string): boolean {
103111
* first segment names no page. Returns "" for a target that is the environment root itself.
104112
*
105113
* A bare name uses its landing path. Deeper segments are grafted onto the prefix, so
106-
* `/deeplink/waitpoints/waitpoint_123` reaches the token that actually lives at
114+
* `/_/waitpoints/waitpoint_123` reaches the token that actually lives at
107115
* `/waitpoints/tokens/waitpoint_123`. A suffix that already spells out a path under the prefix is
108-
* kept as it was written, so both `/deeplink/waitpoints/waitpoint_123` and the longhand
109-
* `/deeplink/waitpoints/tokens/waitpoint_123` arrive at the same place.
116+
* kept as it was written, so both `/_/waitpoints/waitpoint_123` and the longhand
117+
* `/_/waitpoints/tokens/waitpoint_123` arrive at the same place.
110118
*
111119
* `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an
112120
* `encodeURIComponent` pass here would double-encode every id that contains an escape.
113121
*
114-
* The name is matched case-insensitively, to the same end as the prefix in `deeplinkSuffix`:
115-
* `/env/{env}/APIKeys` would have matched its route, so `/deeplink/APIKeys` should reach it rather
116-
* than falling through to the environment root. So is the written-out prefix, which is why the
117-
* comparison is against the lowercased path rather than the path itself — a prefix can be more than
118-
* one segment (`waitpoints/tokens`), and reading only `Tokens` as a segment of its own would graft
119-
* the prefix on top of it and produce `waitpoints/tokens/Tokens/{id}`.
122+
* The name is matched case-insensitively because React Router's route matching is: it compiles
123+
* every path with the `i` flag unless the route opts into `caseSensitive`, so `/env/{env}/APIKeys`
124+
* would have matched its route, and `/_/APIKeys` should reach it rather than falling through to the
125+
* environment root. So is the written-out prefix, which is why the comparison is against the
126+
* lowercased path rather than the path itself — a prefix can be more than one segment
127+
* (`waitpoints/tokens`), and reading only `Tokens` as a segment of its own would graft the prefix
128+
* on top of it and produce `waitpoints/tokens/Tokens/{id}`.
120129
*
121130
* The prefix comes back in the map's own spelling and everything past it exactly as written, since
122131
* folding the case of a task or run id would break the link far more thoroughly than the miss this

0 commit comments

Comments
 (0)