Skip to content

Commit 52c9f6d

Browse files
emmerichclaudeSamyPesse
authored
Fix an issue with link resolution in cross-space reusable content. (#3392)
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Samy Pessé <samypesse@gmail.com>
1 parent dd65987 commit 52c9f6d

File tree

5 files changed

+163
-118
lines changed

5 files changed

+163
-118
lines changed

packages/gitbook/src/components/DocumentView/ReusableContent.tsx

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
import type { DocumentBlockReusableContent } from '@gitbook/api';
22

3-
import { resolveContentRef } from '@/lib/references';
4-
5-
import type { GitBookSpaceContext } from '@/lib/context';
63
import { getDataOrNull } from '@/lib/data';
4+
import { resolveContentRef } from '@/lib/references';
75
import type { BlockProps } from './Block';
86
import { UnwrappedBlocks } from './Blocks';
97

@@ -34,7 +32,7 @@ export async function ReusableContent(props: BlockProps<DocumentBlockReusableCon
3432

3533
const document = await getDataOrNull(
3634
dataFetcher.getDocument({
37-
spaceId: reusableContent.space.id,
35+
spaceId: reusableContent.context.space.id,
3836
documentId: reusableContent.revisionReusableContent.document,
3937
})
4038
);
@@ -43,35 +41,14 @@ export async function ReusableContent(props: BlockProps<DocumentBlockReusableCon
4341
return null;
4442
}
4543

46-
// Create a new context for reusable content block, including
47-
// the data fetcher with the token from the block meta and the correct
48-
// space and revision pointers.
49-
const reusableContentContext: GitBookSpaceContext =
50-
context.contentContext.space.id === reusableContent.space.id
51-
? context.contentContext
52-
: {
53-
...context.contentContext,
54-
dataFetcher,
55-
space: reusableContent.space,
56-
// When the reusable content is in a different space, we don't resolve relative links to pages
57-
// as this space might not be part of the current site.
58-
// In the future, we might expand the logic to look up the space from the list of all spaces in the site
59-
// and adapt the relative links to point to the correct variant.
60-
revision: {
61-
...reusableContent.revision,
62-
pages: [], // TODO: check with Steven
63-
},
64-
shareKey: undefined,
65-
};
66-
6744
return (
6845
<UnwrappedBlocks
6946
nodes={document.nodes}
7047
document={document}
7148
ancestorBlocks={[...ancestorBlocks, block]}
7249
context={{
7350
...context,
74-
contentContext: reusableContentContext,
51+
contentContext: reusableContent.context,
7552
}}
7653
/>
7754
);

packages/gitbook/src/lib/links.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from 'bun:test';
2-
import { createLinker } from './links';
2+
import { createLinker, linkerWithAbsoluteURLs, linkerWithOtherSpaceBasePath } from './links';
33

44
const root = createLinker({
55
host: 'docs.company.com',
@@ -94,3 +94,24 @@ describe('toLinkForContent', () => {
9494
);
9595
});
9696
});
97+
98+
describe('linkerWithAbsoluteURLs', () => {
99+
it('should return a new linker that always returns absolute URLs', () => {
100+
const absoluteLinker = linkerWithAbsoluteURLs(root);
101+
expect(absoluteLinker.toPathInSpace('some/path')).toBe(
102+
'https://docs.company.com/some/path'
103+
);
104+
expect(absoluteLinker.toPathInSite('some/path')).toBe('https://docs.company.com/some/path');
105+
});
106+
});
107+
108+
describe('linkerWithOtherSpaceBasePath', () => {
109+
it('should return a new linker that resolves links relative to a new spaceBasePath in the current site', () => {
110+
const otherSpaceBasePathLinker = linkerWithOtherSpaceBasePath(root, {
111+
spaceBasePath: '/section/variant',
112+
});
113+
expect(otherSpaceBasePathLinker.toPathInSpace('some/path')).toBe(
114+
'/section/variant/some/path'
115+
);
116+
});
117+
});

packages/gitbook/src/lib/links.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ export function createLinker(
9292
return normalizedPath.slice(servedOn.siteBasePath.length);
9393
},
9494

95+
toPathForPage({ pages, page, anchor }) {
96+
return linker.toPathInSpace(getPagePath(pages, page)) + (anchor ? `#${anchor}` : '');
97+
},
98+
9599
toAbsoluteURL(absolutePath: string): string {
96100
// If the path is already a full URL, we return it as is.
97101
if (URL.canParse(absolutePath)) {
@@ -105,10 +109,6 @@ export function createLinker(
105109
return `${servedOn.protocol ?? 'https:'}//${joinPaths(servedOn.host, absolutePath)}`;
106110
},
107111

108-
toPathForPage({ pages, page, anchor }) {
109-
return linker.toPathInSpace(getPagePath(pages, page)) + (anchor ? `#${anchor}` : '');
110-
},
111-
112112
toLinkForContent(rawURL: string): string {
113113
const url = new URL(rawURL);
114114

@@ -125,6 +125,38 @@ export function createLinker(
125125
return linker;
126126
}
127127

128+
/**
129+
* Create a new linker that always returns absolute URLs.
130+
*/
131+
export function linkerWithAbsoluteURLs(linker: GitBookLinker): GitBookLinker {
132+
return {
133+
...linker,
134+
toPathInSpace: (path) => linker.toAbsoluteURL(linker.toPathInSpace(path)),
135+
toPathInSite: (path) => linker.toAbsoluteURL(linker.toPathInSite(path)),
136+
toPathForPage: (input) => linker.toAbsoluteURL(linker.toPathForPage(input)),
137+
};
138+
}
139+
140+
/**
141+
* Create a new linker that resolves links relative to a new spaceBasePath in the current site.
142+
*/
143+
export function linkerWithOtherSpaceBasePath(
144+
linker: GitBookLinker,
145+
{ spaceBasePath }: { spaceBasePath: string }
146+
): GitBookLinker {
147+
const newLinker: GitBookLinker = {
148+
...linker,
149+
toPathInSpace(relativePath: string): string {
150+
return joinPaths(spaceBasePath, relativePath);
151+
},
152+
toPathForPage({ pages, page, anchor }) {
153+
return newLinker.toPathInSpace(getPagePath(pages, page)) + (anchor ? `#${anchor}` : '');
154+
},
155+
};
156+
157+
return newLinker;
158+
}
159+
128160
function joinPaths(prefix: string, path: string): string {
129161
const prefixPath = prefix.endsWith('/') ? prefix : `${prefix}/`;
130162
const suffixPath = path.startsWith('/') ? path.slice(1) : path;

0 commit comments

Comments
 (0)