From 7599253c4b075522de23305126ec0e8a8c572023 Mon Sep 17 00:00:00 2001 From: Steven Hall Date: Sat, 14 Jun 2025 11:01:18 +0100 Subject: [PATCH 1/5] Fix reusable content cross-space page reference resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add parentSpaceContext to GitBookSpaceContext type for tracking parent space information - Update ReusableContent component to include parent space context when rendering content from different spaces - Enhance resolveContentRef to check parent space when page resolution fails in current space - Add comprehensive unit tests for the new parent space fallback functionality This resolves broken links to pages in the parent space of reusable content when used across different sites. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- packages/gitbook-v2/src/lib/context.ts | 8 + .../DocumentView/ReusableContent.tsx | 7 + packages/gitbook/src/lib/references.test.ts | 306 ++++++++++++++++++ packages/gitbook/src/lib/references.tsx | 14 + 4 files changed, 335 insertions(+) create mode 100644 packages/gitbook/src/lib/references.test.ts diff --git a/packages/gitbook-v2/src/lib/context.ts b/packages/gitbook-v2/src/lib/context.ts index ba0f299574..3fcc9b52c3 100644 --- a/packages/gitbook-v2/src/lib/context.ts +++ b/packages/gitbook-v2/src/lib/context.ts @@ -92,6 +92,14 @@ export type GitBookSpaceContext = GitBookBaseContext & { /** Share key of the space. */ shareKey: string | undefined; + + /** Parent space context for reusable content cross-space resolution. */ + parentSpaceContext?: { + space: Space; + revisionId: string; + pages: RevisionPage[]; + shareKey: string | undefined; + }; }; export type SiteSections = { diff --git a/packages/gitbook/src/components/DocumentView/ReusableContent.tsx b/packages/gitbook/src/components/DocumentView/ReusableContent.tsx index ccb66babd4..1e8505420a 100644 --- a/packages/gitbook/src/components/DocumentView/ReusableContent.tsx +++ b/packages/gitbook/src/components/DocumentView/ReusableContent.tsx @@ -60,6 +60,13 @@ export async function ReusableContent(props: BlockProps ({ + getDataOrNull: mock((promise: Promise) => promise.catch(() => null)), + getPageDocument: mock(), + ignoreDataThrownError: mock((promise: Promise) => promise.catch(() => null)), +})); + +mock.module('@v2/lib/context', () => ({ + fetchSpaceContextByIds: mock().mockResolvedValue({ + space: { + id: 'parent-space', + title: 'Parent Space', + urls: { + published: 'https://example.com/space/parent-space', + app: 'https://app.gitbook.com/s/parent-space', + }, + }, + pages: [], + revisionId: 'rev-1', + organizationId: 'org-1', + changeRequest: null, + shareKey: undefined, + dataFetcher: { + getSpace: mock().mockResolvedValue(null), + getPublishedContentSite: mock().mockResolvedValue(null), + }, + linker: { + toPathForPage: mock(({ page }: { page: RevisionPage }) => `/page/${page.id}`), + toAbsoluteURL: mock((url: string) => `https://example.com${url}`), + }, + }), +})); + +mock.module('@v2/lib/links', () => ({ + createLinker: mock().mockReturnValue({ + toAbsoluteURL: mock((url: string) => `https://example.com${url}`), + toPathForPage: mock(({ page }: { page: RevisionPage }) => `/page/${page.id}`), + toLinkForContent: mock((url: string) => url), + }), +})); + +mock.module('./pages', () => ({ + resolvePageId: mockResolvePageId, +})); + +mock.module('./sites', () => ({ + findSiteSpaceById: mock(), +})); + +mock.module('./app', () => ({ + getGitBookAppHref: mock((path: string) => `https://app.gitbook.com${path}`), +})); + +mock.module('./document', () => ({ + getBlockById: mock(), + getBlockTitle: mock(), +})); + +mock.module('../components/PageIcon', () => ({ + PageIcon: mock().mockReturnValue(null), +})); + +import { resolveContentRef } from './references'; + +const createMockSpace = (id: string, title: string): Space => ({ + id, + title, + urls: { + published: `https://example.com/space/${id}`, + app: `https://app.gitbook.com/s/${id}`, + }, + organization: 'org-1', + revision: 'rev-1', +} as Space); + +const createMockPage = (id: string, title: string): RevisionPage => ({ + id, + title, + slug: title.toLowerCase().replace(/\s+/g, '-'), + type: 'page', +} as RevisionPage); + +const createMockContext = ( + space: Space, + pages: RevisionPage[], + parentSpaceContext?: GitBookSpaceContext['parentSpaceContext'] +): GitBookSpaceContext => ({ + organizationId: 'org-1', + space, + changeRequest: null, + revisionId: 'rev-1', + pages, + shareKey: undefined, + dataFetcher: { + getSpace: mock().mockResolvedValue(null), + getPublishedContentSite: mock().mockResolvedValue(null), + } as any, + linker: { + toPathForPage: mock(({ page }: { page: RevisionPage }) => `/page/${page.id}`), + toAbsoluteURL: mock((url: string) => `https://example.com${url}`), + } as any, + parentSpaceContext, +}); + +describe('resolveContentRef - parent space context functionality', () => { + beforeEach(() => { + mockResolvePageId.mockReset(); + }); + + describe('page resolution with parent space context', () => { + it('should resolve page from current space when available', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const parentSpace = createMockSpace('parent-space', 'Parent Space'); + + const currentPages = [createMockPage('page-1', 'Current Page')]; + const parentPages = [createMockPage('page-2', 'Parent Page')]; + + const context = createMockContext(currentSpace, currentPages, { + space: parentSpace, + revisionId: 'rev-1', + pages: parentPages, + shareKey: undefined, + }); + + const contentRef: ContentRef = { + kind: 'page', + page: 'page-1', + }; + + // Mock resolvePageId to return the page from current space + const currentPageResult = { page: currentPages[0], ancestors: [] }; + mockResolvePageId.mockReturnValue(currentPageResult); + + const result = await resolveContentRef(contentRef, context); + + expect(result).not.toBeNull(); + expect(result?.text).toBe('Current Page'); + expect(result?.href).toBe('/page/page-1'); + expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-1'); + }); + + it('should attempt to resolve from parent space when not found in current space', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const parentSpace = createMockSpace('parent-space', 'Parent Space'); + + const currentPages: RevisionPage[] = []; + const parentPages = [createMockPage('page-2', 'Parent Page')]; + + const context = createMockContext(currentSpace, currentPages, { + space: parentSpace, + revisionId: 'rev-1', + pages: parentPages, + shareKey: undefined, + }); + + const contentRef: ContentRef = { + kind: 'page', + page: 'page-2', + }; + + // Mock resolvePageId to return undefined for current space, then return page for parent space + mockResolvePageId + .mockReturnValueOnce(undefined) // First call with current pages + .mockReturnValueOnce({ page: parentPages[0], ancestors: [] }); // Second call with parent pages + + const result = await resolveContentRef(contentRef, context); + + // The function should check both current space and parent space + expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-2'); + expect(mockResolvePageId).toHaveBeenCalledWith(parentPages, 'page-2'); + // Note: resolveContentRefInSpace may call resolvePageId again, so we check for at least 2 calls + expect(mockResolvePageId.mock.calls.length).toBeGreaterThanOrEqual(2); + }); + + it('should return null when page not found in either current or parent space', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const parentSpace = createMockSpace('parent-space', 'Parent Space'); + + const currentPages: RevisionPage[] = []; + const parentPages: RevisionPage[] = []; + + const context = createMockContext(currentSpace, currentPages, { + space: parentSpace, + revisionId: 'rev-1', + pages: parentPages, + shareKey: undefined, + }); + + const contentRef: ContentRef = { + kind: 'page', + page: 'non-existent-page', + }; + + // Mock resolvePageId to return undefined for both spaces + mockResolvePageId.mockReturnValue(undefined); + + const result = await resolveContentRef(contentRef, context); + + expect(result).toBeNull(); + expect(mockResolvePageId).toHaveBeenCalledTimes(2); + }); + + it('should work without parent space context (original behavior)', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const currentPages = [createMockPage('page-1', 'Current Page')]; + + const context = createMockContext(currentSpace, currentPages); + + const contentRef: ContentRef = { + kind: 'page', + page: 'page-1', + }; + + // Mock resolvePageId to return the page + const pageResult = { page: currentPages[0], ancestors: [] }; + mockResolvePageId.mockReturnValue(pageResult); + + const result = await resolveContentRef(contentRef, context); + + expect(result).not.toBeNull(); + expect(result?.text).toBe('Current Page'); + expect(mockResolvePageId).toHaveBeenCalledTimes(1); + expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-1'); + }); + + it('should return null when page not found and no parent space context', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const currentPages: RevisionPage[] = []; + + const context = createMockContext(currentSpace, currentPages); + + const contentRef: ContentRef = { + kind: 'page', + page: 'non-existent-page', + }; + + // Mock resolvePageId to return undefined + mockResolvePageId.mockReturnValue(undefined); + + const result = await resolveContentRef(contentRef, context); + + expect(result).toBeNull(); + expect(mockResolvePageId).toHaveBeenCalledTimes(1); + }); + + it('should handle anchor references with parent space fallback', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const parentSpace = createMockSpace('parent-space', 'Parent Space'); + + const currentPages: RevisionPage[] = []; + const parentPages = [createMockPage('page-2', 'Parent Page')]; + + const context = createMockContext(currentSpace, currentPages, { + space: parentSpace, + revisionId: 'rev-1', + pages: parentPages, + shareKey: undefined, + }); + + const contentRef: ContentRef = { + kind: 'anchor', + page: 'page-2', + anchor: 'section-1', + }; + + // Mock resolvePageId to return undefined for current space, then return page for parent space + mockResolvePageId + .mockReturnValueOnce(undefined) // First call with current pages + .mockReturnValueOnce({ page: parentPages[0], ancestors: [] }); // Second call with parent pages + + const result = await resolveContentRef(contentRef, context); + + // The function should check both current space and parent space + expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-2'); + expect(mockResolvePageId).toHaveBeenCalledWith(parentPages, 'page-2'); + // Note: resolveContentRefInSpace may call resolvePageId again, so we check for at least 2 calls + expect(mockResolvePageId.mock.calls.length).toBeGreaterThanOrEqual(2); + }); + }); + + describe('other content ref types should not be affected', () => { + it('should handle URL references normally', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const context = createMockContext(currentSpace, []); + + const contentRef: ContentRef = { + kind: 'url', + url: 'https://example.com', + }; + + const result = await resolveContentRef(contentRef, context); + + expect(result).not.toBeNull(); + expect(result?.href).toBe('https://example.com'); + expect(result?.text).toBe('https://example.com'); + expect(mockResolvePageId).not.toHaveBeenCalled(); + }); + }); +}); \ No newline at end of file diff --git a/packages/gitbook/src/lib/references.tsx b/packages/gitbook/src/lib/references.tsx index 7362decbe6..7e0e128bef 100644 --- a/packages/gitbook/src/lib/references.tsx +++ b/packages/gitbook/src/lib/references.tsx @@ -83,6 +83,9 @@ export async function resolveContentRef( const activePage = 'page' in context ? context.page : undefined; + const DEBUG = contentRef.kind === 'page'; + DEBUG && console.log(`resolving`, contentRef, context); + switch (contentRef.kind) { case 'url': { return { @@ -124,6 +127,17 @@ export async function resolveContentRef( : undefined : resolvePageId(pages, contentRef.page); + DEBUG && console.log('resolvePageResult', resolvePageResult); + + // If page not found and we have parent space context, try resolving there + if (!resolvePageResult && 'parentSpaceContext' in context && context.parentSpaceContext) { + const parentResult = resolvePageId(context.parentSpaceContext.pages, contentRef.page); + if (parentResult) { + // Return absolute URL to parent space page + return resolveContentRefInSpace(context.parentSpaceContext.space.id, context, contentRef); + } + } + const page = resolvePageResult?.page; const ancestors = resolvePageResult?.ancestors.map((ancestor) => ({ From 0a89373f612fec9a67fbe4b6279b333570164f20 Mon Sep 17 00:00:00 2001 From: Steven Hall Date: Sat, 14 Jun 2025 11:02:46 +0100 Subject: [PATCH 2/5] Add instructions --- packages/gitbook/src/lib/REFERENCES.md | 36 ++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 packages/gitbook/src/lib/REFERENCES.md diff --git a/packages/gitbook/src/lib/REFERENCES.md b/packages/gitbook/src/lib/REFERENCES.md new file mode 100644 index 0000000000..4b684df9a9 --- /dev/null +++ b/packages/gitbook/src/lib/REFERENCES.md @@ -0,0 +1,36 @@ +# Content references + +Content references are a concept in the GitBook API for linking between spaces, pages, anchors, files, and other entities. A definition of a content reference can be found in the OpenAPI spec under `api.ContentRef`. + +Every link in GitBook - even to external URLs - is defined as a content reference. + +Content references can be, by design, relative. We call "resolving a content ref" the process of taking a potentially relative ref along with the current context of the user and producing an absolute link: + +```typescript +async function resolveContentRef(contentRef: api.ContentRef, context: GitBookSpaceContext): Promise; +``` + +Relative content references are resolved against the context: + +```typescript +const ref = { kind: 'page', page: 'my-page-id' }; // a relative ref to a page in the current space +const resolved = await resolveContentRef(ref, { space: 'my-space-id' }); +// resolved.url = ../../my-page-id + +const resolved = await resolveContentRef(ref, { space: 'another-space-id' }); +// resolved == null, because the page does not exist here +``` + +## Reusable Content + +Reusable content presents an interesting challenge for content references: + +- Reusable content belongs to a parent space. +- All content refs defined in the reusable content will be relative to the parent space. For example, for a content ref inside `my-space`, `{ kind: page, page: 'my-page-id' }` refers to the page `my-page-id` inside `my-space`. +- When reusable content is used in pages within the parent space, all links remain relative to the space. +- When reusable content is used across other spaces, we must resolve the refs relative to the containing space (not the parent). + +See the [ReusableContent](../components/DocumentView/ReusableContent.tsx) component for how we construct the `GitBookSpaceContext`, and [resolveContentRef](./references.tsx) for the `resolveContentRef` implementation. + + + From 6a939b5685b67dbcb274f2239f1e1b646998e22a Mon Sep 17 00:00:00 2001 From: Steven Hall Date: Sat, 14 Jun 2025 12:25:48 +0100 Subject: [PATCH 3/5] Unit tests --- packages/gitbook-v2/src/lib/context.ts | 8 - .../DocumentView/ReusableContent.tsx | 7 - packages/gitbook/src/lib/references.test.ts | 360 +++++------------- packages/gitbook/src/lib/references.tsx | 14 - 4 files changed, 92 insertions(+), 297 deletions(-) diff --git a/packages/gitbook-v2/src/lib/context.ts b/packages/gitbook-v2/src/lib/context.ts index 3fcc9b52c3..ba0f299574 100644 --- a/packages/gitbook-v2/src/lib/context.ts +++ b/packages/gitbook-v2/src/lib/context.ts @@ -92,14 +92,6 @@ export type GitBookSpaceContext = GitBookBaseContext & { /** Share key of the space. */ shareKey: string | undefined; - - /** Parent space context for reusable content cross-space resolution. */ - parentSpaceContext?: { - space: Space; - revisionId: string; - pages: RevisionPage[]; - shareKey: string | undefined; - }; }; export type SiteSections = { diff --git a/packages/gitbook/src/components/DocumentView/ReusableContent.tsx b/packages/gitbook/src/components/DocumentView/ReusableContent.tsx index 1e8505420a..ccb66babd4 100644 --- a/packages/gitbook/src/components/DocumentView/ReusableContent.tsx +++ b/packages/gitbook/src/components/DocumentView/ReusableContent.tsx @@ -60,13 +60,6 @@ export async function ReusableContent(props: BlockProps ({ - getDataOrNull: mock((promise: Promise) => promise.catch(() => null)), - getPageDocument: mock(), - ignoreDataThrownError: mock((promise: Promise) => promise.catch(() => null)), -})); - -mock.module('@v2/lib/context', () => ({ - fetchSpaceContextByIds: mock().mockResolvedValue({ - space: { - id: 'parent-space', - title: 'Parent Space', - urls: { - published: 'https://example.com/space/parent-space', - app: 'https://app.gitbook.com/s/parent-space', - }, - }, - pages: [], - revisionId: 'rev-1', - organizationId: 'org-1', - changeRequest: null, - shareKey: undefined, - dataFetcher: { - getSpace: mock().mockResolvedValue(null), - getPublishedContentSite: mock().mockResolvedValue(null), - }, - linker: { - toPathForPage: mock(({ page }: { page: RevisionPage }) => `/page/${page.id}`), - toAbsoluteURL: mock((url: string) => `https://example.com${url}`), - }, - }), -})); +import { resolveContentRef } from './references'; -mock.module('@v2/lib/links', () => ({ - createLinker: mock().mockReturnValue({ - toAbsoluteURL: mock((url: string) => `https://example.com${url}`), - toPathForPage: mock(({ page }: { page: RevisionPage }) => `/page/${page.id}`), - toLinkForContent: mock((url: string) => url), - }), -})); +const NOW = new Date(); -mock.module('./pages', () => ({ - resolvePageId: mockResolvePageId, -})); +describe('resolveContentRef', () => { + it('should resolve a relative page ref', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const currentPages = [createMockPage('page-1', 'Current Page')]; + const context = createMockContext({ space: currentSpace, pages: currentPages }); -mock.module('./sites', () => ({ - findSiteSpaceById: mock(), -})); + const result = await resolveContentRef( + { + kind: 'page', + page: 'page-1', + }, + context + ); -mock.module('./app', () => ({ - getGitBookAppHref: mock((path: string) => `https://app.gitbook.com${path}`), -})); + expect(result).not.toBeNull(); + expect(result?.text).toBe('Current Page'); + expect(result?.href).toBe('/page/page-1'); + }); -mock.module('./document', () => ({ - getBlockById: mock(), - getBlockTitle: mock(), -})); + it('should resolve a url', async () => { + const currentSpace = createMockSpace('current-space', 'Current Space'); + const currentPages = [createMockPage('page-1', 'Current Page')]; + const context = createMockContext({ space: currentSpace, pages: currentPages }); -mock.module('../components/PageIcon', () => ({ - PageIcon: mock().mockReturnValue(null), -})); + const result = await resolveContentRef( + { + kind: 'url', + url: 'https://example.com/some-page', + }, + context + ); -import { resolveContentRef } from './references'; + expect(result).not.toBeNull(); + expect(result?.text).toBe('https://example.com/some-page'); + expect(result?.href).toBe('https://example.com/some-page'); + }); +}); -const createMockSpace = (id: string, title: string): Space => ({ +const createMockSpace = (id: string, title: string): api.Space => ({ + object: 'space', id, title, + visibility: api.ContentVisibility.Public, urls: { + location: `https://api.gitbook.com/s/${id}`, published: `https://example.com/space/${id}`, app: `https://app.gitbook.com/s/${id}`, }, organization: 'org-1', revision: 'rev-1', -} as Space); + emoji: '', + createdAt: NOW.toISOString(), + updatedAt: NOW.toISOString(), + defaultLevel: 'inherit', + comments: 0, + changeRequests: 0, + changeRequestsOpen: 0, + changeRequestsDraft: 0, + permissions: { + view: true, + access: true, + admin: true, + viewInviteLinks: true, + edit: true, + triggerGitSync: true, + comment: true, + merge: true, + review: true, + }, +}); -const createMockPage = (id: string, title: string): RevisionPage => ({ +const createMockPage = (id: string, title: string): api.RevisionPage => ({ id, title, slug: title.toLowerCase().replace(/\s+/g, '-'), - type: 'page', -} as RevisionPage); + kind: 'sheet', + type: 'document', + layout: {}, + urls: { + app: `https://app.gitbook.com/s/${id}/page-1`, + }, + path: '/page-1', + pages: [], + document: { + object: 'document', + data: {}, + nodes: [], + }, +}); const createMockContext = ( - space: Space, - pages: RevisionPage[], - parentSpaceContext?: GitBookSpaceContext['parentSpaceContext'] + context: MandateProps, 'space' | 'pages'> ): GitBookSpaceContext => ({ organizationId: 'org-1', - space, changeRequest: null, revisionId: 'rev-1', - pages, shareKey: undefined, dataFetcher: { getSpace: mock().mockResolvedValue(null), getPublishedContentSite: mock().mockResolvedValue(null), } as any, linker: { - toPathForPage: mock(({ page }: { page: RevisionPage }) => `/page/${page.id}`), + toPathForPage: mock(({ page }: { page: api.RevisionPage }) => `/page/${page.id}`), toAbsoluteURL: mock((url: string) => `https://example.com${url}`), } as any, - parentSpaceContext, + ...context, }); -describe('resolveContentRef - parent space context functionality', () => { - beforeEach(() => { - mockResolvePageId.mockReset(); - }); - - describe('page resolution with parent space context', () => { - it('should resolve page from current space when available', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const parentSpace = createMockSpace('parent-space', 'Parent Space'); - - const currentPages = [createMockPage('page-1', 'Current Page')]; - const parentPages = [createMockPage('page-2', 'Parent Page')]; - - const context = createMockContext(currentSpace, currentPages, { - space: parentSpace, - revisionId: 'rev-1', - pages: parentPages, - shareKey: undefined, - }); - - const contentRef: ContentRef = { - kind: 'page', - page: 'page-1', - }; - - // Mock resolvePageId to return the page from current space - const currentPageResult = { page: currentPages[0], ancestors: [] }; - mockResolvePageId.mockReturnValue(currentPageResult); - - const result = await resolveContentRef(contentRef, context); - - expect(result).not.toBeNull(); - expect(result?.text).toBe('Current Page'); - expect(result?.href).toBe('/page/page-1'); - expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-1'); - }); - - it('should attempt to resolve from parent space when not found in current space', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const parentSpace = createMockSpace('parent-space', 'Parent Space'); - - const currentPages: RevisionPage[] = []; - const parentPages = [createMockPage('page-2', 'Parent Page')]; - - const context = createMockContext(currentSpace, currentPages, { - space: parentSpace, - revisionId: 'rev-1', - pages: parentPages, - shareKey: undefined, - }); - - const contentRef: ContentRef = { - kind: 'page', - page: 'page-2', - }; - - // Mock resolvePageId to return undefined for current space, then return page for parent space - mockResolvePageId - .mockReturnValueOnce(undefined) // First call with current pages - .mockReturnValueOnce({ page: parentPages[0], ancestors: [] }); // Second call with parent pages - - const result = await resolveContentRef(contentRef, context); - - // The function should check both current space and parent space - expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-2'); - expect(mockResolvePageId).toHaveBeenCalledWith(parentPages, 'page-2'); - // Note: resolveContentRefInSpace may call resolvePageId again, so we check for at least 2 calls - expect(mockResolvePageId.mock.calls.length).toBeGreaterThanOrEqual(2); - }); - - it('should return null when page not found in either current or parent space', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const parentSpace = createMockSpace('parent-space', 'Parent Space'); - - const currentPages: RevisionPage[] = []; - const parentPages: RevisionPage[] = []; - - const context = createMockContext(currentSpace, currentPages, { - space: parentSpace, - revisionId: 'rev-1', - pages: parentPages, - shareKey: undefined, - }); - - const contentRef: ContentRef = { - kind: 'page', - page: 'non-existent-page', - }; - - // Mock resolvePageId to return undefined for both spaces - mockResolvePageId.mockReturnValue(undefined); - - const result = await resolveContentRef(contentRef, context); - - expect(result).toBeNull(); - expect(mockResolvePageId).toHaveBeenCalledTimes(2); - }); - - it('should work without parent space context (original behavior)', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const currentPages = [createMockPage('page-1', 'Current Page')]; - - const context = createMockContext(currentSpace, currentPages); - - const contentRef: ContentRef = { - kind: 'page', - page: 'page-1', - }; - - // Mock resolvePageId to return the page - const pageResult = { page: currentPages[0], ancestors: [] }; - mockResolvePageId.mockReturnValue(pageResult); - - const result = await resolveContentRef(contentRef, context); - - expect(result).not.toBeNull(); - expect(result?.text).toBe('Current Page'); - expect(mockResolvePageId).toHaveBeenCalledTimes(1); - expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-1'); - }); - - it('should return null when page not found and no parent space context', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const currentPages: RevisionPage[] = []; - - const context = createMockContext(currentSpace, currentPages); - - const contentRef: ContentRef = { - kind: 'page', - page: 'non-existent-page', - }; - - // Mock resolvePageId to return undefined - mockResolvePageId.mockReturnValue(undefined); - - const result = await resolveContentRef(contentRef, context); - - expect(result).toBeNull(); - expect(mockResolvePageId).toHaveBeenCalledTimes(1); - }); - - it('should handle anchor references with parent space fallback', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const parentSpace = createMockSpace('parent-space', 'Parent Space'); - - const currentPages: RevisionPage[] = []; - const parentPages = [createMockPage('page-2', 'Parent Page')]; - - const context = createMockContext(currentSpace, currentPages, { - space: parentSpace, - revisionId: 'rev-1', - pages: parentPages, - shareKey: undefined, - }); - - const contentRef: ContentRef = { - kind: 'anchor', - page: 'page-2', - anchor: 'section-1', - }; - - // Mock resolvePageId to return undefined for current space, then return page for parent space - mockResolvePageId - .mockReturnValueOnce(undefined) // First call with current pages - .mockReturnValueOnce({ page: parentPages[0], ancestors: [] }); // Second call with parent pages - - const result = await resolveContentRef(contentRef, context); - - // The function should check both current space and parent space - expect(mockResolvePageId).toHaveBeenCalledWith(currentPages, 'page-2'); - expect(mockResolvePageId).toHaveBeenCalledWith(parentPages, 'page-2'); - // Note: resolveContentRefInSpace may call resolvePageId again, so we check for at least 2 calls - expect(mockResolvePageId.mock.calls.length).toBeGreaterThanOrEqual(2); - }); - }); - - describe('other content ref types should not be affected', () => { - it('should handle URL references normally', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const context = createMockContext(currentSpace, []); - - const contentRef: ContentRef = { - kind: 'url', - url: 'https://example.com', - }; - - const result = await resolveContentRef(contentRef, context); - - expect(result).not.toBeNull(); - expect(result?.href).toBe('https://example.com'); - expect(result?.text).toBe('https://example.com'); - expect(mockResolvePageId).not.toHaveBeenCalled(); - }); - }); -}); \ No newline at end of file +/** + * Type to make optional properties on a object mandatory. + * + * interface SomeObject { + * uid: string; + * price: number | null; + * location?: string; + * } + * + * type ValuableObject = MandateProps; + */ +type MandateProps = T & { + [MK in K]-?: NonNullable; +}; diff --git a/packages/gitbook/src/lib/references.tsx b/packages/gitbook/src/lib/references.tsx index 7e0e128bef..7362decbe6 100644 --- a/packages/gitbook/src/lib/references.tsx +++ b/packages/gitbook/src/lib/references.tsx @@ -83,9 +83,6 @@ export async function resolveContentRef( const activePage = 'page' in context ? context.page : undefined; - const DEBUG = contentRef.kind === 'page'; - DEBUG && console.log(`resolving`, contentRef, context); - switch (contentRef.kind) { case 'url': { return { @@ -127,17 +124,6 @@ export async function resolveContentRef( : undefined : resolvePageId(pages, contentRef.page); - DEBUG && console.log('resolvePageResult', resolvePageResult); - - // If page not found and we have parent space context, try resolving there - if (!resolvePageResult && 'parentSpaceContext' in context && context.parentSpaceContext) { - const parentResult = resolvePageId(context.parentSpaceContext.pages, contentRef.page); - if (parentResult) { - // Return absolute URL to parent space page - return resolveContentRefInSpace(context.parentSpaceContext.space.id, context, contentRef); - } - } - const page = resolvePageResult?.page; const ancestors = resolvePageResult?.ancestors.map((ancestor) => ({ From 52f2cace6f92b7d694e82b744144adaef8030de9 Mon Sep 17 00:00:00 2001 From: Steven Hall Date: Sat, 14 Jun 2025 12:33:06 +0100 Subject: [PATCH 4/5] Fix test file --- packages/gitbook/src/lib/references.test.ts | 32 +++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/gitbook/src/lib/references.test.ts b/packages/gitbook/src/lib/references.test.ts index 2a76b66912..a6ee5ed94a 100644 --- a/packages/gitbook/src/lib/references.test.ts +++ b/packages/gitbook/src/lib/references.test.ts @@ -8,8 +8,8 @@ const NOW = new Date(); describe('resolveContentRef', () => { it('should resolve a relative page ref', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const currentPages = [createMockPage('page-1', 'Current Page')]; + const currentSpace = createMockSpace({ id: 'current-space' }); + const currentPages = [createMockPage({ id: 'page-1' })]; const context = createMockContext({ space: currentSpace, pages: currentPages }); const result = await resolveContentRef( @@ -26,8 +26,8 @@ describe('resolveContentRef', () => { }); it('should resolve a url', async () => { - const currentSpace = createMockSpace('current-space', 'Current Space'); - const currentPages = [createMockPage('page-1', 'Current Page')]; + const currentSpace = createMockSpace({ id: 'current-space' }); + const currentPages = [createMockPage({ id: 'page-1' })]; const context = createMockContext({ space: currentSpace, pages: currentPages }); const result = await resolveContentRef( @@ -44,15 +44,14 @@ describe('resolveContentRef', () => { }); }); -const createMockSpace = (id: string, title: string): api.Space => ({ +const createMockSpace = (space: MandateProps, 'id'>): api.Space => ({ object: 'space', - id, - title, + title: 'My Space', visibility: api.ContentVisibility.Public, urls: { - location: `https://api.gitbook.com/s/${id}`, - published: `https://example.com/space/${id}`, - app: `https://app.gitbook.com/s/${id}`, + location: `https://api.gitbook.com/s/${space.id}`, + published: `https://example.com/space/${space.id}`, + app: `https://app.gitbook.com/s/${space.id}`, }, organization: 'org-1', revision: 'rev-1', @@ -75,17 +74,19 @@ const createMockSpace = (id: string, title: string): api.Space => ({ merge: true, review: true, }, + ...space, }); -const createMockPage = (id: string, title: string): api.RevisionPage => ({ - id, - title, - slug: title.toLowerCase().replace(/\s+/g, '-'), +const createMockPage = ( + page: MandateProps, 'id'> +): api.RevisionPageDocument => ({ + title: 'Page 1', + slug: 'page-1', kind: 'sheet', type: 'document', layout: {}, urls: { - app: `https://app.gitbook.com/s/${id}/page-1`, + app: `https://app.gitbook.com/s/${page.id}/page-1`, }, path: '/page-1', pages: [], @@ -94,6 +95,7 @@ const createMockPage = (id: string, title: string): api.RevisionPage => ({ data: {}, nodes: [], }, + ...page, }); const createMockContext = ( From fac5fcfbc2f3db64cbbc860fe35089f8e75ee5f2 Mon Sep 17 00:00:00 2001 From: Steven Hall Date: Sat, 14 Jun 2025 13:54:16 +0100 Subject: [PATCH 5/5] Add failing test --- packages/gitbook/src/lib/references.test.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/gitbook/src/lib/references.test.ts b/packages/gitbook/src/lib/references.test.ts index a6ee5ed94a..bd5630c21c 100644 --- a/packages/gitbook/src/lib/references.test.ts +++ b/packages/gitbook/src/lib/references.test.ts @@ -21,7 +21,7 @@ describe('resolveContentRef', () => { ); expect(result).not.toBeNull(); - expect(result?.text).toBe('Current Page'); + expect(result?.text).toBe('Page 1'); expect(result?.href).toBe('/page/page-1'); }); @@ -44,6 +44,25 @@ describe('resolveContentRef', () => { }); }); +describe('resolveContentRef with reusable content', () => { + it('should resolve a relative page ref with reusable content', async () => { + const rcSpace = createMockSpace({ id: 'rc-parent-space' }); + const context = createMockContext({ space: rcSpace, pages: [] }); + + const result = await resolveContentRef( + { + kind: 'page', + page: 'page-1', + }, + context + ); + + expect(result).not.toBeNull(); + expect(result?.text).toBe('Current Page'); + expect(result?.href).toBe('/page/page-1'); + }); +}); + const createMockSpace = (space: MandateProps, 'id'>): api.Space => ({ object: 'space', title: 'My Space',