Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jsii): submodule READMEs don't have literate examples #3347

Merged
merged 4 commits into from Jan 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 23 additions & 13 deletions packages/jsii/lib/assembler.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -793,18 +792,13 @@ export class Assembler implements Emitter {
*/
async function loadSubmoduleReadMe(
submoduleMain: string,
projectRoot: string,
): Promise<SubmoduleSpec['readme']> {
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);
}
}

Expand Down Expand Up @@ -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'),
};
}
26 changes: 22 additions & 4 deletions packages/jsii/lib/literate.ts
Expand Up @@ -71,7 +71,14 @@ export function typescriptSourceToMarkdown(
return markdownLines;
}

export type FileLoader = (relativePath: string) => string[] | Promise<string[]>;
export interface LoadedFile {
readonly fullPath: string;
readonly lines: string[];
}

export type FileLoader = (
relativePath: string,
) => LoadedFile | Promise<LoadedFile>;

/**
* Given MarkDown source, find source files to include and render
Expand All @@ -84,6 +91,7 @@ export type FileLoader = (relativePath: string) => string[] | Promise<string[]>;
export async function includeAndRenderExamples(
lines: string[],
loader: FileLoader,
projectRoot: string,
): Promise<string[]> {
const ret: string[] = [];

Expand All @@ -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=${toUnixPath(path.relative(projectRoot, fullPath))}`,
]);
ret.push(...imported);
} else {
ret.push(line);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -215,3 +229,7 @@ function markdownify(
}
}
}

function toUnixPath(x: string) {
return x.replace(/\\/g, '/');
}
11 changes: 9 additions & 2 deletions packages/jsii/test/literate.test.ts
Expand Up @@ -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',
Expand Down
24 changes: 24 additions & 0 deletions packages/jsii/test/submodules.test.ts
Expand Up @@ -60,6 +60,30 @@ 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[])(
Expand Down