Skip to content

Commit

Permalink
fix: Handle non-unique anchor ids (#303)
Browse files Browse the repository at this point in the history
  • Loading branch information
tgreyuk committed Apr 9, 2022
1 parent 1035fd4 commit 787748f
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 27 deletions.
2 changes: 1 addition & 1 deletion packages/typedoc-plugin-markdown/package.json
Expand Up @@ -9,7 +9,7 @@
"scripts": {
"lint": "eslint ./src --ext .ts",
"prepublishOnly": "yarn run lint && yarn run build && yarn run test",
"build": "rm -rf dist && tsc && copyfiles --up 1 ./src/**/*.hbs ./dist/",
"build": "rimraf dist && tsc && copyfiles --up 1 ./src/**/*.hbs ./dist/",
"pretest": "yarn run demo:md && markdownlint ./demo/md",
"test": "jest --colors",
"build-and-test": "yarn run build && yarn run test",
Expand Down
23 changes: 19 additions & 4 deletions packages/typedoc-plugin-markdown/src/theme.ts
Expand Up @@ -12,9 +12,7 @@ import {
UrlMapping,
} from 'typedoc';
import { getKindPlural } from './groups';

import { NavigationItem } from './navigation-item';

import {
indexTemplate,
reflectionMemberTemplate,
Expand Down Expand Up @@ -44,6 +42,7 @@ export class MarkdownTheme extends Theme {
project?: ProjectReflection;
reflection?: DeclarationReflection;
location!: string;
anchorMap: Record<string, string[]> = {};

static URL_PREFIX = /^(http|ftp)s?:\/\//;

Expand Down Expand Up @@ -127,6 +126,7 @@ export class MarkdownTheme extends Theme {
reflection.url = url;
reflection.hasOwnDocument = true;
}

for (const child of reflection.children || []) {
if (mapping.isLeaf) {
this.applyAnchorUrl(child, reflection);
Expand Down Expand Up @@ -160,9 +160,24 @@ export class MarkdownTheme extends Theme {
}

applyAnchorUrl(reflection: Reflection, container: Reflection) {
if (!reflection.url || !MarkdownTheme.URL_PREFIX.test(reflection.url)) {
if (
container.url &&
(!reflection.url || !MarkdownTheme.URL_PREFIX.test(reflection.url))
) {
const reflectionId = reflection.name.toLowerCase();
const anchor = this.toAnchorRef(reflectionId);

this.anchorMap[container.url]
? this.anchorMap[container.url].push(reflectionId)
: (this.anchorMap[container.url] = [reflectionId]);

const count = this.anchorMap[container.url].filter(
(id) => id === reflectionId,
)?.length;

const anchor = this.toAnchorRef(
reflectionId + (count > 1 ? '-' + (count - 1).toString() : ''),
);

reflection.url = container.url + '#' + anchor;
reflection.anchor = anchor;
reflection.hasOwnDocument = false;
Expand Down
Expand Up @@ -148,16 +148,16 @@ exports[`Declarations: should compile object literal declaration 1`] = `
#### Type declaration
| Name | Type |
| :------ | :------ |
| \`valueA\` | \`number\` |
| \`valueB\` | \`boolean\` |
| \`valueC\` | {} |
| \`valueX\` | { \`valueA\`: \`number\`[] ; \`valueZ\`: \`string\` = 'foo' } |
| \`valueX.valueA\` | \`number\`[] |
| \`valueX.valueZ\` | \`string\` |
| \`valueY\` | () => \`string\` |
| \`valueZ\` | \`string\` |
| Name | Type | Description |
| :------ | :------ | :------ |
| \`valueA\` | \`number\` | Comment for valueA |
| \`valueB\` | \`boolean\` | - |
| \`valueC\` | {} | - |
| \`valueX\` | { \`valueA\`: \`number\`[] ; \`valueZ\`: \`string\` = 'foo' } | Comment for valueX |
| \`valueX.valueA\` | \`number\`[] | - |
| \`valueX.valueZ\` | \`string\` | - |
| \`valueY\` | () => \`string\` | Comment for value Y |
| \`valueZ\` | \`string\` | Comment for valueZ |
[partial: member.sources]
"
Expand Down
Expand Up @@ -5,23 +5,27 @@ exports[`TOC: (default) should display toc for class' 1`] = `
### Constructors
- [constructor](Breadcrumbs.md#constructor)
- [constructor](SomeClass.md#constructor)
### Methods
- [someMethod](Breadcrumbs.md#somemethod)
- [someMethod](SomeClass.md#somemethod)
"
`;

exports[`TOC: (default) should display toc for module' 1`] = `
"## Table of contents
### Constructors
### Classes
- [constructor](Breadcrumbs.md#constructor)
- [SomeClass](SomeClass.md)
### Methods
### Type aliases
- [answer](../modules.md#answer)
### Variables
- [someMethod](Breadcrumbs.md#somemethod)
- [answer](../modules.md#answer-1)
"
`;
11 changes: 5 additions & 6 deletions packages/typedoc-plugin-markdown/test/specs/toc.spec.ts
@@ -1,6 +1,5 @@
import * as Handlebars from 'handlebars';
import { Reflection } from 'typedoc';

import { TestApp } from '../test-app';

describe(`TOC:`, () => {
Expand All @@ -10,10 +9,10 @@ describe(`TOC:`, () => {
describe(`(default)`, () => {
let testApp: TestApp;
beforeAll(async () => {
testApp = new TestApp(['breadcrumbs.ts']);
testApp = new TestApp(['toc.ts']);
await testApp.bootstrap();
moduleReflection = testApp.project.children[0];
classReflection = testApp.project.findReflectionByName('Breadcrumbs');
moduleReflection = testApp.project;
classReflection = testApp.project.findReflectionByName('SomeClass');
});

test(`should display toc for module'`, () => {
Expand All @@ -31,10 +30,10 @@ describe(`TOC:`, () => {
describe(`(hideInPageToc)`, () => {
let testApp: TestApp;
beforeAll(async () => {
testApp = new TestApp(['breadcrumbs.ts']);
testApp = new TestApp(['toc.ts']);
await testApp.bootstrap({ hideInPageTOC: true });
moduleReflection = testApp.project.children[0];
classReflection = testApp.project.findReflectionByName('Breadcrumbs');
classReflection = testApp.project.findReflectionByName('SomeClass');
});

test(`should not display toc for class'`, () => {
Expand Down
1 change: 1 addition & 0 deletions packages/typedoc-plugin-markdown/test/stubs/src/index.ts
Expand Up @@ -10,4 +10,5 @@ export * as reflections from './reflections';
export * as signatures from './signatures';
export * as sources from './sources';
export * as theme from './theme';
export * as toc from './toc';
export * as types from './types';
9 changes: 9 additions & 0 deletions packages/typedoc-plugin-markdown/test/stubs/src/toc.ts
@@ -0,0 +1,9 @@
export type answer = 'yes' | 'no';

export const answer = 'yes';

export class SomeClass {
someMethod() {
return true;
}
}

2 comments on commit 787748f

@panva
Copy link

@panva panva commented on 787748f Apr 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has likely not taken into consideration allReflectionsHaveOwnDocument and in doing so broken links that are unique per document but not when combined, or maybe something else, dunno really.

Feel free to test

git clone --branch v0.3.0 https://github.com/panva/oauth4webapi
npm clean-install
npm upgrade
npm run docs
// check git changes..., all `*-1` anchors are wrong.

Edit:

the wrong links are ones that are also present in AuthorizationServer.mtls_endpoint_aliases?.[_endpoint], disregarding the fact that they're nested object properties.

@tgreyuk
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok.. thanks will fix a.s.a.p..

Please sign in to comment.