Skip to content

Commit

Permalink
redesign rscPath encoding (#951)
Browse files Browse the repository at this point in the history
Technically breaking.
  • Loading branch information
dai-shi authored Oct 7, 2024
1 parent a71896c commit 978ec3f
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 54 deletions.
2 changes: 1 addition & 1 deletion e2e/fixtures/broken-links/public/serve.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"redirects": [
{ "source": "/redirect", "destination": "/exists" },
{ "source": "/RSC/redirect.txt", "destination": "/RSC/exists.txt" },
{ "source": "/RSC/R/redirect.txt", "destination": "/RSC/R/exists.txt" },
{ "source": "/broken-redirect", "destination": "/broken" }
]
}
2 changes: 1 addition & 1 deletion e2e/fixtures/ssr-catch-error/src/middleware/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const validateMiddleware: Middleware = () => {
return async (ctx, next) => {
if (
ctx.req.url.pathname === '/invalid' ||
ctx.req.url.pathname.startsWith(`/${rscBase}/invalid`)
ctx.req.url.pathname.startsWith(`/${rscBase}/R/invalid`)
) {
ctx.res.status = 401;
ctx.res.body = stringToStream('Unauthorized');
Expand Down
4 changes: 2 additions & 2 deletions e2e/partial-builds.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ test.describe(`partial builds`, () => {

test('does not change pages that already exist', async () => {
const htmlBefore = statSync(`${cwd}/dist/public/page/a/index.html`);
const rscBefore = statSync(`${cwd}/dist/public/RSC/page/a.txt`);
const rscBefore = statSync(`${cwd}/dist/public/RSC/R/page/a.txt`);
execSync(`node ${waku} build --experimental-partial`, {
cwd,
env: { ...process.env, PAGES: 'a,b' },
});
const htmlAfter = statSync(`${cwd}/dist/public/page/a/index.html`);
const rscAfter = statSync(`${cwd}/dist/public/RSC/page/a.txt`);
const rscAfter = statSync(`${cwd}/dist/public/RSC/R/page/a.txt`);
expect(htmlBefore.mtimeMs).toBe(htmlAfter.mtimeMs);
expect(rscBefore.mtimeMs).toBe(rscAfter.mtimeMs);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { readdir, writeFile } from 'node:fs/promises';
import { existsSync, readFileSync } from 'node:fs';
import { SRC_ENTRIES, EXTENSIONS } from '../constants.js';
import { joinPath } from '../utils/path.js';
import { getRscPath } from '../../router/common.js';

const SRC_PAGES = 'pages';

Expand Down Expand Up @@ -44,7 +43,7 @@ export function getImportModuleNames(filePaths: string[]): {
identifier = `${identifier}_${moduleNameCount[identifier]}`;
}
try {
moduleNames[getRscPath(filePath)] = identifier;
moduleNames[filePath.replace(/^\//, '')] = identifier;
} catch (e) {
console.log(e);
}
Expand Down Expand Up @@ -128,7 +127,7 @@ export const fsRouterTypegenPlugin = (opts: { srcDir: string }): Plugin => {

for (const filePath of filePaths) {
// where to import the component from
const src = getRscPath(filePath);
const src = filePath.replace(/^\//, '');
const hasGetConfig = fileExportsGetConfig(filePath);

if (filePath.endsWith('/_layout.tsx')) {
Expand Down
4 changes: 2 additions & 2 deletions packages/waku/src/lib/plugins/vite-plugin-rsc-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ export function rscTransformPlugin(
}
for (const [k, v] of Object.entries(opts.clientEntryFiles)) {
if (v === id) {
return `@id/${k}.js`;
return k;
}
}
throw new Error('client id not found: ' + id);
Expand All @@ -610,7 +610,7 @@ export function rscTransformPlugin(
}
for (const [k, v] of Object.entries(opts.serverEntryFiles)) {
if (v === id) {
return `@id/${k}.js`;
return k;
}
}
throw new Error('server id not found: ' + id);
Expand Down
14 changes: 3 additions & 11 deletions packages/waku/src/lib/renderers/rsc-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ export const SERVER_MODULE_MAP = {
} as const;

const resolveClientEntryForPrd = (id: string, config: { basePath: string }) => {
if (!id.startsWith('@id/')) {
throw new Error('Unexpected client entry in PRD');
}
return config.basePath + id.slice('@id/'.length);
return config.basePath + id + '.js';
};

export type RenderRscArgs = {
Expand Down Expand Up @@ -116,9 +113,7 @@ export async function renderRsc(
{
get(_target, encodedId: string) {
const [fileId, name] = encodedId.split('#') as [string, string];
const id = fileId.startsWith('@id/')
? fileId.slice('@id/'.length)
: filePathToFileURL(fileId);
const id = isDev ? filePathToFileURL(fileId) : fileId + '.js';
return { id, chunks: [id], name, async: true };
},
},
Expand Down Expand Up @@ -221,10 +216,7 @@ export async function renderRsc(
if (isDev) {
mod = await opts.loadServerModuleRsc(filePathToFileURL(fileId));
} else {
if (!fileId.startsWith('@id/')) {
throw new Error('Unexpected server entry in PRD');
}
mod = await loadModule(fileId.slice('@id/'.length));
mod = await loadModule(fileId + '.js');
}
const fn = mod[name] || mod;
return renderWithContextWithFunc(context, fn, args);
Expand Down
39 changes: 23 additions & 16 deletions packages/waku/src/lib/renderers/utils.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,46 @@
// This file should not include Node specific code.

export const encodeRscPath = (rscPath: string) => {
if (rscPath === '') {
return 'index.txt';
if (rscPath.startsWith('_')) {
throw new Error('rscPath must not start with `_`: ' + rscPath);
}
if (rscPath.endsWith('_')) {
throw new Error('rscPath must not end with `_`: ' + rscPath);
}
if (rscPath === 'index') {
throw new Error('rscPath should not be `index`');
if (rscPath === '') {
rscPath = '_';
}
if (rscPath.startsWith('/')) {
throw new Error('rscPath should not start with `/`');
rscPath = '_' + rscPath;
}
if (rscPath.endsWith('/')) {
throw new Error('rscPath should not end with `/`');
rscPath += '_';
}
return rscPath + '.txt';
};

export const decodeRscPath = (encodedRscPath: string) => {
if (encodedRscPath === 'index.txt') {
return '';
export const decodeRscPath = (rscPath: string) => {
if (!rscPath.endsWith('.txt')) {
const err = new Error('Invalid encoded rscPath');
(err as any).statusCode = 400;
throw err;
}
rscPath = rscPath.slice(0, -'.txt'.length);
if (rscPath.startsWith('_')) {
rscPath = rscPath.slice(1);
}
if (encodedRscPath?.endsWith('.txt')) {
return encodedRscPath.slice(0, -'.txt'.length);
if (rscPath.endsWith('_')) {
rscPath = rscPath.slice(0, -1);
}
const err = new Error('Invalid encoded rscPath');
(err as any).statusCode = 400;
throw err;
return rscPath;
};

const FUNC_PREFIX = 'FUNC_';
const FUNC_PREFIX = 'F/';

export const encodeFuncId = (funcId: string) => {
const [file, name] = funcId.split('#') as [string, string];
if (name.includes('/')) {
throw new Error('Unsupported function name');
throw new Error('Function name must not include `/`: ' + name);
}
return FUNC_PREFIX + file + '/' + name;
};
Expand Down
10 changes: 5 additions & 5 deletions packages/waku/src/router/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import type {
import { fetchRsc, prefetchRsc, Root, Slot, useRefetch } from '../client.js';
import {
getComponentIds,
getRscPath,
encodeRoutePath,
SHOULD_SKIP_ID,
ROUTE_ID,
HAS404_ID,
Expand Down Expand Up @@ -375,7 +375,7 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => {
if (componentIds.every((id) => skip.includes(id))) {
return; // everything is skipped
}
const rscPath = getRscPath(route.path);
const rscPath = encodeRoutePath(route.path);
if (!skipRefetch) {
refetch(rscPath, JSON.stringify({ query: route.query, skip }));
}
Expand Down Expand Up @@ -406,7 +406,7 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => {
if (componentIds.every((id) => skip.includes(id))) {
return; // everything is cached
}
const rscPath = getRscPath(route.path);
const rscPath = encodeRoutePath(route.path);
prefetchRsc(rscPath, JSON.stringify({ query: route.query, skip }));
(globalThis as any).__WAKU_ROUTER_PREFETCH__?.(route.path);
},
Expand Down Expand Up @@ -488,7 +488,7 @@ const DEFAULT_ROUTER_DATA: RouterData = [];

export function Router({ routerData = DEFAULT_ROUTER_DATA }) {
const route = parseRouteFromLocation();
const initialRscPath = getRscPath(route.path);
const initialRscPath = encodeRoutePath(route.path);
const unstable_enhanceCreateData =
(
createData: (
Expand All @@ -501,7 +501,7 @@ export function Router({ routerData = DEFAULT_ROUTER_DATA }) {
if (response.status === 404 && has404) {
// HACK this is still an experimental logic. It's very fragile.
// FIXME we should cache it if 404.txt is static.
return fetchRsc(getRscPath('/404'));
return fetchRsc(encodeRoutePath('/404'));
}
const data = createData(responsePromise);
Promise.resolve(data)
Expand Down
23 changes: 17 additions & 6 deletions packages/waku/src/router/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,29 @@ export function getComponentIds(path: string): readonly string[] {
return Array.from(idSet);
}

export function getRscPath(path: string): string {
const ROUTE_PREFIX = 'R';

export function encodeRoutePath(path: string): string {
if (!path.startsWith('/')) {
throw new Error('Path should start with `/`');
throw new Error('Path must start with `/`: ' + path);
}
if (path === '/') {
return ROUTE_PREFIX + '/_root';
}
return path.slice(1);
if (path.endsWith('/')) {
throw new Error('Path must not end with `/`: ' + path);
}
return ROUTE_PREFIX + path;
}

export function parseRscPath(rscPath: string): string {
if (rscPath.startsWith('/')) {
export function decodeRoutePath(rscPath: string): string {
if (!rscPath.startsWith(ROUTE_PREFIX)) {
throw new Error('rscPath should not start with `/`');
}
return '/' + rscPath;
if (rscPath === ROUTE_PREFIX + '/_root') {
return '/';
}
return rscPath.slice(ROUTE_PREFIX.length);
}

// It starts with "/" to avoid conflicting with normal component ids.
Expand Down
14 changes: 7 additions & 7 deletions packages/waku/src/router/define-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import type {
import { Children, Slot } from '../client.js';
import {
getComponentIds,
getRscPath,
parseRscPath,
encodeRoutePath,
decodeRoutePath,
SHOULD_SKIP_ID,
ROUTE_ID,
HAS404_ID,
Expand Down Expand Up @@ -120,7 +120,7 @@ export function unstable_defineRouter(
};
};
const renderEntries: RenderEntries = async (rscPath, { rscParams }) => {
const pathname = parseRscPath(rscPath);
const pathname = decodeRoutePath(rscPath);
const pathStatus = await existsPath(pathname);
if (!pathStatus.found) {
return null;
Expand Down Expand Up @@ -190,7 +190,7 @@ export function unstable_defineRouter(
return;
}
const pathname = '/' + pathSpec.map(({ name }) => name).join('/');
const rscPath = getRscPath(pathname);
const rscPath = encodeRoutePath(pathname);
path2moduleIds[pattern] = await unstable_collectClientModules(rscPath);
}),
);
Expand All @@ -210,7 +210,7 @@ globalThis.__WAKU_ROUTER_PREFETCH__ = (path) => {
const entries: BuildConfig[number]['entries'] = [];
if (pathSpec.every(({ type }) => type === 'literal')) {
const pathname = '/' + pathSpec.map(({ name }) => name).join('/');
const rscPath = getRscPath(pathname);
const rscPath = encodeRoutePath(pathname);
entries.push({ rscPath, isStatic });
}
buildConfig.push({
Expand Down Expand Up @@ -240,7 +240,7 @@ globalThis.__WAKU_ROUTER_PREFETCH__ = (path) => {
}
}
const componentIds = getComponentIds(pathname);
const rscPath = getRscPath(pathname);
const rscPath = encodeRoutePath(pathname);
const html = createElement(
ServerRouter as FunctionComponent<
Omit<ComponentProps<typeof ServerRouter>, 'children'>
Expand All @@ -266,6 +266,6 @@ export function unstable_rerenderRoute(
query?: string,
skip?: string[], // TODO this is too hard to use
) {
const rscPath = getRscPath(pathname);
const rscPath = encodeRoutePath(pathname);
rerender(rscPath, { query, skip });
}
83 changes: 83 additions & 0 deletions packages/waku/tests/rscPath.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { test, describe, expect } from 'vitest';
import {
encodeRscPath,
decodeRscPath,
encodeFuncId,
decodeFuncId,
} from '../src/lib/renderers/utils.js';
import { encodeRoutePath, decodeRoutePath } from '../src/router/common.js';

describe('encodeRscPath', () => {
test('encodes rscPath', () => {
expect(encodeRscPath('')).toBe('_.txt');
expect(encodeRscPath('foo')).toBe('foo.txt');
expect(encodeRscPath('foo/')).toBe('foo/_.txt');
expect(encodeRscPath('/foo')).toBe('_/foo.txt');
expect(encodeRscPath('/foo/')).toBe('_/foo/_.txt');
});

test('throws on invalid rscPath', () => {
expect(() => encodeRscPath('_foo')).toThrow();
expect(() => encodeRscPath('foo_')).toThrow();
expect(() => encodeRscPath('_foo_')).toThrow();
});
});

describe('decodeRscPath', () => {
test('decodes rscPath', () => {
expect(decodeRscPath('_.txt')).toBe('');
expect(decodeRscPath('foo.txt')).toBe('foo');
expect(decodeRscPath('foo/_.txt')).toBe('foo/');
expect(decodeRscPath('_/foo.txt')).toBe('/foo');
expect(decodeRscPath('_/foo/_.txt')).toBe('/foo/');
});

test('throws on invalid rscPath', () => {
expect(() => decodeRscPath('foo')).toThrow();
});
});

describe('encodeFuncId', () => {
test('encodes funcId', () => {
expect(encodeFuncId('foo#bar')).toBe('F/foo/bar');
});

test('throws on invalid funcId', () => {
expect(() => encodeFuncId('foo#bar/baz')).toThrow();
});
});

describe('decodeFuncId', () => {
test('decodes funcId', () => {
expect(decodeFuncId('F/foo/bar')).toBe('foo#bar');
});

test('returns null on invalid funcId', () => {
expect(decodeFuncId('foo/bar')).toBe(null);
});
});

describe('encodeRoutePath', () => {
test('encodes routePath', () => {
expect(encodeRoutePath('/')).toBe('R/_root');
expect(encodeRoutePath('/foo')).toBe('R/foo');
expect(encodeRoutePath('/foo/bar')).toBe('R/foo/bar');
});

test('throws on invalid routePath', () => {
expect(() => encodeRoutePath('foo')).toThrow();
expect(() => encodeRoutePath('/foo/')).toThrow();
});
});

describe('decodeRoutePath', () => {
test('decodes routePath', () => {
expect(decodeRoutePath('R/_root')).toBe('/');
expect(decodeRoutePath('R/foo')).toBe('/foo');
expect(decodeRoutePath('R/foo/bar')).toBe('/foo/bar');
});

test('throws on invalid routePath', () => {
expect(() => decodeRoutePath('foo')).toThrow();
});
});

0 comments on commit 978ec3f

Please sign in to comment.