diff --git a/packages/waku/src/router/client.ts b/packages/waku/src/router/client.ts index c446cfd91..5011ce4af 100644 --- a/packages/waku/src/router/client.ts +++ b/packages/waku/src/router/client.ts @@ -16,7 +16,6 @@ import { import type { ComponentProps, FunctionComponent, - MutableRefObject, ReactNode, AnchorHTMLAttributes, ReactElement, @@ -27,10 +26,10 @@ import { prefetchRSC, Root, Slot, useRefetch } from '../client.js'; import { getComponentIds, getInputString, - SHOULD_SKIP_ID, + COMPONENT_CONFIGS_ID, LOCATION_ID, } from './common.js'; -import type { RouteProps, ShouldSkip } from './common.js'; +import type { RouteProps, ComponentConfigs } from './common.js'; declare global { interface ImportMeta { @@ -62,7 +61,7 @@ const parseRoute = (url: URL): RouteProps => { type ChangeRoute = ( route: RouteProps, options?: { - checkCache?: boolean; + unstable_checkCache?: boolean; // TODO this doesn't work 100% correctly skipRefetch?: boolean; }, ) => void; @@ -235,59 +234,36 @@ export function Link({ } const getSkipList = ( - shouldSkip: ShouldSkip | undefined, + componentConfigs: ComponentConfigs | undefined, componentIds: readonly string[], - route: RouteProps, - cached: Record, ): string[] => { - const shouldSkipObj = Object.fromEntries(shouldSkip || []); return componentIds.filter((id) => { - const prevProps = cached[id]; - if (!prevProps) { - return false; - } - const shouldCheck = shouldSkipObj[id]; - if (!shouldCheck) { - return false; - } - if (shouldCheck[0] && route.path !== prevProps.path) { - return false; + const config = componentConfigs?.[id]; + if (!config) { + return false; // no config } - if (shouldCheck[1] && route.query !== prevProps.query) { - return false; + const [render] = config; + if (!render) { + return true; // no component } - return true; + return render === 'static'; // static component }); }; -const equalRouteProps = (a: RouteProps, b: RouteProps) => { - if (a.path !== b.path) { - return false; - } - if (a.query !== b.query) { - return false; - } - return true; -}; - const RouterSlot = ({ - route, routerData, - cachedRef, id, fallback, children, }: { - route: RouteProps; routerData: RouterData; - cachedRef: MutableRefObject>; id: string; fallback?: ReactNode; children?: ReactNode; }) => { const unstable_shouldRenderPrev = (_err: unknown) => { const shouldSkip = routerData[0]; - const skip = getSkipList(shouldSkip, [id], route, cachedRef.current); + const skip = getSkipList(shouldSkip, [id]); return skip.length > 0; }; return createElement( @@ -329,9 +305,7 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { const componentIds = getComponentIds(route.path); - const [cached, setCached] = useState>(() => { - return Object.fromEntries(componentIds.map((id) => [id, route])); - }); + const [cached, setCached] = useState(() => new Set(componentIds)); const cachedRef = useRef(cached); useEffect(() => { cachedRef.current = cached; @@ -339,27 +313,19 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { const changeRoute: ChangeRoute = useCallback( (route, options) => { - const { checkCache, skipRefetch } = options || {}; + const { unstable_checkCache, skipRefetch } = options || {}; startTransition(() => { setRoute(route); }); const componentIds = getComponentIds(route.path); if ( - checkCache && - componentIds.every((id) => { - const cachedLoc = cachedRef.current[id]; - return cachedLoc && equalRouteProps(cachedLoc, route); - }) + unstable_checkCache && + componentIds.every((id) => cachedRef.current.has(id)) ) { return; // everything is cached } const shouldSkip = routerData[0]; - const skip = getSkipList( - shouldSkip, - componentIds, - route, - cachedRef.current, - ); + const skip = getSkipList(shouldSkip, componentIds); if (componentIds.every((id) => skip.includes(id))) { return; // everything is skipped } @@ -368,14 +334,9 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { refetch(input, JSON.stringify({ query: route.query, skip })); } startTransition(() => { - setCached((prev) => ({ - ...prev, - ...Object.fromEntries( - componentIds.flatMap((id) => - skip.includes(id) ? [] : [[id, route]], - ), - ), - })); + // HACK this is just guessing the waku/client behavior + // There should be a better way with exposing something from waku/client + setCached((prev) => new Set([...prev, ...componentIds])); }); }, [refetch, routerData], @@ -385,12 +346,7 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { (route) => { const componentIds = getComponentIds(route.path); const shouldSkip = routerData[0]; - const skip = getSkipList( - shouldSkip, - componentIds, - route, - cachedRef.current, - ); + const skip = getSkipList(shouldSkip, componentIds); if (componentIds.every((id) => skip.includes(id))) { return; // everything is cached } @@ -404,7 +360,9 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { useEffect(() => { const callback = () => { const route = parseRoute(new URL(window.location.href)); - changeRoute(route, { checkCache: true }); + // TODO unstable_checkCache only works for going back just once + // can we recover state from the History API? + changeRoute(route, { unstable_checkCache: true }); }; window.addEventListener('popstate', callback); return () => { @@ -448,11 +406,7 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { const children = componentIds.reduceRight( (acc: ReactNode, id) => - createElement( - RouterSlot, - { route, routerData, cachedRef, id, fallback: acc }, - acc, - ), + createElement(RouterSlot, { routerData, id, fallback: acc }, acc), null, ); @@ -465,7 +419,7 @@ const InnerRouter = ({ routerData }: { routerData: RouterData }) => { // Note: The router data must be a stable mutable object (array). type RouterData = [ - shouldSkip?: ShouldSkip, + componentConfigs?: ComponentConfigs, locationListners?: Set<(path: string, query: string) => void>, ]; @@ -478,24 +432,21 @@ export function Router({ routerData = DEFAULT_ROUTER_DATA }) { Promise.resolve(data) .then((data) => { if (data && typeof data === 'object') { - // We need to process SHOULD_SKIP_ID before LOCATION_ID - if (SHOULD_SKIP_ID in data) { - // TODO replacing the whole array is not ideal - routerData[0] = data[SHOULD_SKIP_ID] as ShouldSkip; + // We need to process COMPONENT_CONFIGS_ID before LOCATION_ID + if (COMPONENT_CONFIGS_ID in data) { + routerData[0] = { + ...routerData[0], + ...Object.fromEntries(data[COMPONENT_CONFIGS_ID] as any), + }; } if (LOCATION_ID in data) { - const [pathname, searchParamsString] = data[LOCATION_ID] as [ - string, - string, - ]; + const [pathname, query] = data[LOCATION_ID] as [string, string]; // FIXME this check here seems ad-hoc (less readable code) if ( window.location.pathname !== pathname || - window.location.search.replace(/^\?/, '') !== searchParamsString + window.location.search.replace(/^\?/, '') !== query ) { - routerData[1]?.forEach((listener) => - listener(pathname, searchParamsString), - ); + routerData[1]?.forEach((listener) => listener(pathname, query)); } } } diff --git a/packages/waku/src/router/common.ts b/packages/waku/src/router/common.ts index 14405bd64..03727f5f1 100644 --- a/packages/waku/src/router/common.ts +++ b/packages/waku/src/router/common.ts @@ -30,16 +30,11 @@ export function parseInputString(input: string): string { } // It starts with "/" to avoid conflicting with normal component ids. -export const SHOULD_SKIP_ID = '/SHOULD_SKIP'; +export const COMPONENT_CONFIGS_ID = '/CONFIGS'; // It starts with "/" to avoid conflicting with normal component ids. export const LOCATION_ID = '/LOCATION'; -// TODO revisit shouldSkip API -export type ShouldSkip = (readonly [ - componentId: string, - readonly [ - path?: boolean, // if we compare path - query?: boolean, // if we compare query - ], -])[]; +export type ComponentConfigs = { + [componentId: string]: [render?: 'static' | 'dynamic']; +}; diff --git a/packages/waku/src/router/create-pages.ts b/packages/waku/src/router/create-pages.ts index 9b7732bfb..642edf03c 100644 --- a/packages/waku/src/router/create-pages.ts +++ b/packages/waku/src/router/create-pages.ts @@ -393,11 +393,11 @@ export function createPages( } return paths; }, - async (id, { unstable_setShouldSkip, unstable_buildConfig }) => { + async (id, { unstable_setComponentConfig, unstable_buildConfig }) => { await configure(unstable_buildConfig); const staticComponent = staticComponentMap.get(id); if (staticComponent) { - unstable_setShouldSkip([]); + unstable_setComponentConfig('static'); return staticComponent; } for (const [_, [pathSpec, Component]] of dynamicPagePathMap) { @@ -407,12 +407,12 @@ export function createPages( ); if (mapping) { if (Object.keys(mapping).length === 0) { - unstable_setShouldSkip(); + unstable_setComponentConfig('dynamic'); return Component; } const WrappedComponent = (props: Record) => createElement(Component, { ...props, ...mapping }); - unstable_setShouldSkip(); + unstable_setComponentConfig('dynamic'); return WrappedComponent; } } @@ -424,7 +424,7 @@ export function createPages( if (mapping) { const WrappedComponent = (props: Record) => createElement(Component, { ...props, ...mapping }); - unstable_setShouldSkip(); + unstable_setComponentConfig('dynamic'); return WrappedComponent; } } @@ -437,11 +437,11 @@ export function createPages( if (Object.keys(mapping).length) { throw new Error('[Bug] layout should not have slugs'); } - unstable_setShouldSkip(); + unstable_setComponentConfig('dynamic'); return Component; } } - unstable_setShouldSkip([]); // negative cache + unstable_setComponentConfig(); // negative cache return null; // not found }, ); diff --git a/packages/waku/src/router/define-router.ts b/packages/waku/src/router/define-router.ts index e0263f40d..2a0e33e9d 100644 --- a/packages/waku/src/router/define-router.ts +++ b/packages/waku/src/router/define-router.ts @@ -13,10 +13,10 @@ import { getComponentIds, getInputString, parseInputString, - SHOULD_SKIP_ID, + COMPONENT_CONFIGS_ID, LOCATION_ID, } from './common.js'; -import type { RouteProps, ShouldSkip } from './common.js'; +import type { RouteProps, ComponentConfigs } from './common.js'; import { getPathMapping } from '../lib/utils/path.js'; import type { PathSpec } from '../lib/utils/path.js'; import { ServerRouter } from './client.js'; @@ -25,8 +25,6 @@ type RoutePropsForLayout = Omit & { children: ReactNode; }; -type ShouldSkipValue = ShouldSkip[number][1]; - const safeJsonParse = (str: unknown) => { if (typeof str === 'string') { try { @@ -54,8 +52,9 @@ export function unstable_defineRouter( getComponent: ( componentId: string, // "**/layout" or "**/page" options: { - // TODO setShouldSkip API is too hard to understand - unstable_setShouldSkip: (val?: ShouldSkipValue) => void; + unstable_setComponentConfig: ( + ...args: [render?: 'static' | 'dynamic'] + ) => void; unstable_buildConfig: BuildConfig | undefined; }, ) => Promise< @@ -117,9 +116,7 @@ export function unstable_defineRouter( if ((await existsPath(pathname, buildConfig))[0] === 'NOT_FOUND') { return null; } - const shouldSkipObj: { - [componentId: ShouldSkip[number][0]]: ShouldSkip[number][1]; - } = {}; + const componentConfigs: ComponentConfigs = {}; const parsedParams = safeJsonParse(params); @@ -135,15 +132,13 @@ export function unstable_defineRouter( if (skip?.includes(id)) { return []; } - const setShoudSkip = (val?: ShouldSkipValue) => { - if (val) { - shouldSkipObj[id] = val; - } else { - delete shouldSkipObj[id]; - } + const setComponentConfig = ( + ...args: [render?: 'static' | 'dynamic'] + ) => { + componentConfigs[id] = args; }; const component = await getComponent(id, { - unstable_setShouldSkip: setShoudSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: buildConfig, }); if (!component) { @@ -163,7 +158,7 @@ export function unstable_defineRouter( }), ) ).flat(); - entries.push([SHOULD_SKIP_ID, Object.entries(shouldSkipObj)]); + entries.push([COMPONENT_CONFIGS_ID, Object.entries(componentConfigs)]); entries.push([LOCATION_ID, [pathname, query]]); return Object.fromEntries(entries); }; @@ -256,7 +251,7 @@ globalThis.__WAKU_ROUTER_PREFETCH__ = (path) => { export function unstable_redirect( pathname: string, query?: string, - skip?: string[], + skip?: string[], // FIXME how could we specify this?? ) { const input = getInputString(pathname); rerender(input, { query, skip }); diff --git a/packages/waku/tests/create-pages.test.ts b/packages/waku/tests/create-pages.test.ts index 0df9b1782..ef7afd254 100644 --- a/packages/waku/tests/create-pages.test.ts +++ b/packages/waku/tests/create-pages.test.ts @@ -235,16 +235,16 @@ describe('createPages', () => { }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); expect( await getComponent!('test/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }), ).toBe(TestPage); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith([]); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('static'); }); it('creates a simple dynamic page', async () => { @@ -271,15 +271,15 @@ describe('createPages', () => { pattern: '^/test$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); expect( await getComponent('test/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }), ).toBe(TestPage); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith(); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('dynamic'); }); it('creates a simple static page with a layout', async () => { @@ -314,25 +314,25 @@ describe('createPages', () => { }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); expect( await getComponent('test/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }), ).toBe(TestPage); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith([]); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('static'); - const setShouldSkipLayout = vi.fn(); + const setComponentConfigLayout = vi.fn(); expect( await getComponent('layout', { - unstable_setShouldSkip: setShouldSkipLayout, + unstable_setComponentConfig: setComponentConfigLayout, unstable_buildConfig: undefined, }), ).toBe(TestLayout); - expect(setShouldSkipLayout).toHaveBeenCalledTimes(1); - expect(setShouldSkipLayout).toHaveBeenCalledWith([]); + expect(setComponentConfigLayout).toHaveBeenCalledTimes(1); + expect(setComponentConfigLayout).toHaveBeenCalledWith('static'); }); it('creates a simple dynamic page with a layout', async () => { @@ -367,25 +367,25 @@ describe('createPages', () => { }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); expect( await getComponent('test/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }), ).toBe(TestPage); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith(); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('dynamic'); - const setShouldSkipLayout = vi.fn(); + const setComponentConfigLayout = vi.fn(); expect( await getComponent('layout', { - unstable_setShouldSkip: setShouldSkipLayout, + unstable_setComponentConfig: setComponentConfigLayout, unstable_buildConfig: undefined, }), ).toBe(TestLayout); - expect(setShouldSkipLayout).toHaveBeenCalledTimes(1); - expect(setShouldSkipLayout).toHaveBeenCalledWith(); + expect(setComponentConfigLayout).toHaveBeenCalledTimes(1); + expect(setComponentConfigLayout).toHaveBeenCalledWith('dynamic'); }); it('creates a nested static page', async () => { @@ -416,15 +416,15 @@ describe('createPages', () => { pattern: '^/test/nested$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); expect( await getComponent('test/nested/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }), ).toBe(TestPage); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith([]); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('static'); }); it('creates a nested dynamic page', async () => { @@ -455,15 +455,15 @@ describe('createPages', () => { pattern: '^/test/nested$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); expect( await getComponent('test/nested/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }), ).toBe(TestPage); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith(); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('dynamic'); }); it('creates a static page with slugs', async () => { @@ -522,14 +522,14 @@ describe('createPages', () => { pattern: '^/test/([^/]+)/([^/]+)$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); const WrappedComponent = await getComponent('test/w/x/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }); assert(WrappedComponent); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith([]); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('static'); renderToString(createElement(WrappedComponent as any)); expect(TestPage).toHaveBeenCalledTimes(1); expect(TestPage).toHaveBeenCalledWith({ a: 'w', b: 'x' }, undefined); @@ -567,14 +567,14 @@ describe('createPages', () => { pattern: '^/test/([^/]+)/([^/]+)$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); const WrappedComponent = await getComponent('test/w/x/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }); assert(WrappedComponent); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith(); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('dynamic'); renderToString(createElement(WrappedComponent as any)); expect(TestPage).toHaveBeenCalledTimes(1); expect(TestPage).toHaveBeenCalledWith({ a: 'w', b: 'x' }, undefined); @@ -613,14 +613,14 @@ describe('createPages', () => { pattern: '^/test/(.*)$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); const WrappedComponent = await getComponent('test/a/b/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }); assert(WrappedComponent); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith([]); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('static'); renderToString(createElement(WrappedComponent as any)); expect(TestPage).toHaveBeenCalledTimes(1); expect(TestPage).toHaveBeenCalledWith({ path: ['a', 'b'] }, undefined); @@ -654,14 +654,14 @@ describe('createPages', () => { pattern: '^/test/(.*)$', }, ]); - const setShouldSkip = vi.fn(); + const setComponentConfig = vi.fn(); const WrappedComponent = await getComponent('test/a/b/page', { - unstable_setShouldSkip: setShouldSkip, + unstable_setComponentConfig: setComponentConfig, unstable_buildConfig: undefined, }); assert(WrappedComponent); - expect(setShouldSkip).toHaveBeenCalledTimes(1); - expect(setShouldSkip).toHaveBeenCalledWith(); + expect(setComponentConfig).toHaveBeenCalledTimes(1); + expect(setComponentConfig).toHaveBeenCalledWith('dynamic'); renderToString(createElement(WrappedComponent as any)); expect(TestPage).toHaveBeenCalledTimes(1); expect(TestPage).toHaveBeenCalledWith({ path: ['a', 'b'] }, undefined);