From 00825266c06a78c474beea1ef623acef6e03eac0 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 21 Jan 2022 17:14:19 +0100 Subject: [PATCH 1/4] fix(jsii): submodule READMEs don't have literate examples The code literally was not there to extract literate examples for submodule READMEs. Fixes aws/aws-cdk#18589. --- packages/jsii/lib/assembler.ts | 36 +++++++++++++++++---------- packages/jsii/lib/literate.ts | 22 +++++++++++++--- packages/jsii/test/literate.test.ts | 11 ++++++-- packages/jsii/test/submodules.test.ts | 23 +++++++++++++++++ 4 files changed, 73 insertions(+), 19 deletions(-) diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index fad2c3090c..8d8d0f79d3 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -320,11 +320,7 @@ export class Assembler implements Emitter { return undefined; } const readmePath = path.join(this.projectInfo.projectRoot, fileName); - const renderedLines = await literate.includeAndRenderExamples( - await literate.loadFromFile(readmePath), - literate.fileSystemLoader(this.projectInfo.projectRoot), - ); - return { markdown: renderedLines.join('\n') }; + return loadAndRenderReadme(readmePath, this.projectInfo.projectRoot); } function _loadDocs(this: Assembler): spec.Docs | undefined { @@ -729,7 +725,10 @@ export class Assembler implements Emitter { symbol, ); const targets = await loadSubmoduleTargetConfig(sourceFile.fileName); - const readme = await loadSubmoduleReadMe(sourceFile.fileName); + const readme = await loadSubmoduleReadMe( + sourceFile.fileName, + this.projectInfo.projectRoot, + ); this._submodules.set(symbol, { fqn, @@ -793,18 +792,13 @@ export class Assembler implements Emitter { */ async function loadSubmoduleReadMe( submoduleMain: string, + projectRoot: string, ): Promise { const fileBase = path.basename(submoduleMain).replace(/(\.d)?\.ts$/, ''); const readMeName = fileBase === 'index' ? `README.md` : `${fileBase}.README.md`; const fullPath = path.join(path.dirname(submoduleMain), readMeName); - - if (!(await fs.pathExists(fullPath))) { - return undefined; - } - return { - markdown: await fs.readFile(fullPath, { encoding: 'utf-8' }), - }; + return loadAndRenderReadme(fullPath, projectRoot); } } @@ -3481,3 +3475,19 @@ function isUnder(file: string, dir: string): boolean { const relative = path.relative(dir, file); return !relative.startsWith(path.sep) && !relative.startsWith('..'); } + +async function loadAndRenderReadme(readmePath: string, projectRoot: string) { + if (!(await fs.pathExists(readmePath))) { + return undefined; + } + + return { + markdown: ( + await literate.includeAndRenderExamples( + await literate.loadFromFile(readmePath), + literate.fileSystemLoader(path.dirname(readmePath)), + projectRoot, + ) + ).join('\n'), + }; +} diff --git a/packages/jsii/lib/literate.ts b/packages/jsii/lib/literate.ts index 2ebf048708..c1064e4374 100644 --- a/packages/jsii/lib/literate.ts +++ b/packages/jsii/lib/literate.ts @@ -71,7 +71,14 @@ export function typescriptSourceToMarkdown( return markdownLines; } -export type FileLoader = (relativePath: string) => string[] | Promise; +export interface LoadedFile { + readonly fullPath: string; + readonly lines: string[]; +} + +export type FileLoader = ( + relativePath: string, +) => LoadedFile | Promise; /** * Given MarkDown source, find source files to include and render @@ -84,6 +91,7 @@ export type FileLoader = (relativePath: string) => string[] | Promise; export async function includeAndRenderExamples( lines: string[], loader: FileLoader, + projectRoot: string, ): Promise { const ret: string[] = []; @@ -94,9 +102,12 @@ export async function includeAndRenderExamples( // Found an include const filename = m[2]; // eslint-disable-next-line no-await-in-loop - const source = await loader(filename); + const { lines: source, fullPath } = await loader(filename); // 'lit' source attribute will make snippet compiler know to extract the same source - const imported = typescriptSourceToMarkdown(source, [`lit=${filename}`]); + // Needs to be relative to the project root. + const imported = typescriptSourceToMarkdown(source, [ + `lit=${path.relative(projectRoot, fullPath)}`, + ]); ret.push(...imported); } else { ret.push(line); @@ -125,7 +136,10 @@ export function contentToLines(content: string): string[] { * Return a file system loader given a base directory */ export function fileSystemLoader(directory: string): FileLoader { - return (fileName) => loadFromFile(path.resolve(directory, fileName)); + return async (fileName) => { + const fullPath = path.resolve(directory, fileName); + return { fullPath, lines: await loadFromFile(fullPath) }; + }; } const RELEVANT_TAG = '/// !show'; diff --git a/packages/jsii/test/literate.test.ts b/packages/jsii/test/literate.test.ts index 74d44ddf5d..d34bd549ef 100644 --- a/packages/jsii/test/literate.test.ts +++ b/packages/jsii/test/literate.test.ts @@ -99,10 +99,17 @@ test('can do example inclusion', async () => { const fakeLoader = (fileName: string) => { expect(fileName).toBe('test/something.lit.ts'); - return ['const x = 1;', '/// This is how we print x', 'console.log(x);']; + return { + fullPath: fileName, + lines: ['const x = 1;', '/// This is how we print x', 'console.log(x);'], + }; }; - const rendered = await includeAndRenderExamples(inputMarkDown, fakeLoader); + const rendered = await includeAndRenderExamples( + inputMarkDown, + fakeLoader, + '.', + ); expect(rendered).toEqual([ 'This is a preamble', diff --git a/packages/jsii/test/submodules.test.ts b/packages/jsii/test/submodules.test.ts index a924a24137..692b39e6a9 100644 --- a/packages/jsii/test/submodules.test.ts +++ b/packages/jsii/test/submodules.test.ts @@ -60,6 +60,29 @@ test('submodules loaded from directories can have targets', async () => { ); }); +test('submodule READMEs can have literate source references', async () => { + const assembly = await sourceToAssemblyHelper({ + 'index.ts': 'export * as submodule from "./subdir"', + 'subdir/index.ts': 'export class Foo { }', + 'subdir/README.md': 'This is the README\n\n[includable](./test/includeme.lit.ts)', + 'subdir/test/includeme.lit.ts': '// Include me', + }); + + expect(assembly.submodules!['testpkg.submodule']).toEqual( + expect.objectContaining({ + readme: { + markdown: [ + 'This is the README', + '', + '```ts lit=subdir/test/includeme.lit.ts', + '// Include me', + '```', + ].join('\n'), + }, + }), + ); +}); + type ImportStyle = 'directly' | 'as namespace' | 'with alias'; test.each(['directly', 'as namespace', 'with alias'] as ImportStyle[])( From 4b3a08b54033e9d26991ce0518c5f46f8d55ed02 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Jan 2022 10:53:30 +0100 Subject: [PATCH 2/4] Linter fix --- packages/jsii/test/submodules.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/jsii/test/submodules.test.ts b/packages/jsii/test/submodules.test.ts index 692b39e6a9..244e5e8d58 100644 --- a/packages/jsii/test/submodules.test.ts +++ b/packages/jsii/test/submodules.test.ts @@ -64,7 +64,8 @@ test('submodule READMEs can have literate source references', async () => { const assembly = await sourceToAssemblyHelper({ 'index.ts': 'export * as submodule from "./subdir"', 'subdir/index.ts': 'export class Foo { }', - 'subdir/README.md': 'This is the README\n\n[includable](./test/includeme.lit.ts)', + 'subdir/README.md': + 'This is the README\n\n[includable](./test/includeme.lit.ts)', 'subdir/test/includeme.lit.ts': '// Include me', }); From d738f64fe40a1aef6eec2ce895ee9c2630fa4dbf Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Jan 2022 12:02:04 +0100 Subject: [PATCH 3/4] Use posix path-join --- packages/jsii/lib/literate.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsii/lib/literate.ts b/packages/jsii/lib/literate.ts index c1064e4374..8ef0b5b9d1 100644 --- a/packages/jsii/lib/literate.ts +++ b/packages/jsii/lib/literate.ts @@ -106,7 +106,7 @@ export async function includeAndRenderExamples( // 'lit' source attribute will make snippet compiler know to extract the same source // Needs to be relative to the project root. const imported = typescriptSourceToMarkdown(source, [ - `lit=${path.relative(projectRoot, fullPath)}`, + `lit=${path.posix.relative(projectRoot, fullPath)}`, ]); ret.push(...imported); } else { From ac9fb3ef0175bf37f467920a88136559986a773d Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Jan 2022 12:56:40 +0100 Subject: [PATCH 4/4] Needs to be different --- packages/jsii/lib/literate.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/jsii/lib/literate.ts b/packages/jsii/lib/literate.ts index 8ef0b5b9d1..5d6b812989 100644 --- a/packages/jsii/lib/literate.ts +++ b/packages/jsii/lib/literate.ts @@ -106,7 +106,7 @@ export async function includeAndRenderExamples( // 'lit' source attribute will make snippet compiler know to extract the same source // Needs to be relative to the project root. const imported = typescriptSourceToMarkdown(source, [ - `lit=${path.posix.relative(projectRoot, fullPath)}`, + `lit=${toUnixPath(path.relative(projectRoot, fullPath))}`, ]); ret.push(...imported); } else { @@ -229,3 +229,7 @@ function markdownify( } } } + +function toUnixPath(x: string) { + return x.replace(/\\/g, '/'); +}