Skip to content

Commit

Permalink
Fix: Heading ID CI flakiness (#7141)
Browse files Browse the repository at this point in the history
* feat: use `ctx` object instead of leaky global

* test: heading IDs stale caches

* chore: changeset
  • Loading branch information
bholmesdev committed May 19, 2023
1 parent 1473737 commit a9e1cd7
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 43 deletions.
5 changes: 5 additions & 0 deletions .changeset/eleven-tables-speak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/markdoc': patch
---

Fix inconsistent Markdoc heading IDs for documents with the same headings.
12 changes: 5 additions & 7 deletions packages/integrations/markdoc/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { emitESMImage } from 'astro/assets';
import { bold, red, yellow } from 'kleur/colors';
import type * as rollup from 'rollup';
import { loadMarkdocConfig, type MarkdocConfigResult } from './load-config.js';
import { applyDefaultConfig } from './runtime.js';
import { setupConfig } from './runtime.js';

type SetupHookParams = HookParameters<'astro:config:setup'> & {
// `contentEntryType` is not a public API
Expand Down Expand Up @@ -52,7 +52,7 @@ export default function markdocIntegration(legacyConfig?: any): AstroIntegration
async getRenderModule({ entry, viteId }) {
const ast = Markdoc.parse(entry.body);
const pluginContext = this;
const markdocConfig = applyDefaultConfig(userMarkdocConfig, entry);
const markdocConfig = setupConfig(userMarkdocConfig, entry);

const validationErrors = Markdoc.validate(ast, markdocConfig).filter((e) => {
return (
Expand Down Expand Up @@ -90,7 +90,7 @@ export default function markdocIntegration(legacyConfig?: any): AstroIntegration

const res = `import { jsx as h } from 'astro/jsx-runtime';
import { Renderer } from '@astrojs/markdoc/components';
import { collectHeadings, applyDefaultConfig, Markdoc, headingSlugger } from '@astrojs/markdoc/runtime';
import { collectHeadings, setupConfig, Markdoc } from '@astrojs/markdoc/runtime';
import * as entry from ${JSON.stringify(viteId + '?astroContentCollectionEntry')};
${
markdocConfigResult
Expand All @@ -113,16 +113,14 @@ export function getHeadings() {
instead of the Content component. Would remove double-transform and unlock variable resolution in heading slugs. */
''
}
headingSlugger.reset();
const headingConfig = userConfig.nodes?.heading;
const config = applyDefaultConfig(headingConfig ? { nodes: { heading: headingConfig } } : {}, entry);
const config = setupConfig(headingConfig ? { nodes: { heading: headingConfig } } : {}, entry);
const ast = Markdoc.Ast.fromJSON(stringifiedAst);
const content = Markdoc.transform(ast, config);
return collectHeadings(Array.isArray(content) ? content : content.children);
}
export async function Content (props) {
headingSlugger.reset();
const config = applyDefaultConfig({
const config = setupConfig({
...userConfig,
variables: { ...userConfig.variables, ...props },
}, entry);
Expand Down
32 changes: 27 additions & 5 deletions packages/integrations/markdoc/src/nodes/heading.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
import Markdoc, { type RenderableTreeNode, type Schema } from '@markdoc/markdoc';
import Markdoc, { type RenderableTreeNode, type Schema, type ConfigType } from '@markdoc/markdoc';
import Slugger from 'github-slugger';
import { getTextContent } from '../runtime.js';

export const headingSlugger = new Slugger();
type ConfigTypeWithCtx = ConfigType & {
// TODO: decide on `ctx` as a convention for config merging
ctx: {
headingSlugger: Slugger;
};
};

function getSlug(attributes: Record<string, any>, children: RenderableTreeNode[]): string {
function getSlug(
attributes: Record<string, any>,
children: RenderableTreeNode[],
headingSlugger: Slugger
): string {
if (attributes.id && typeof attributes.id === 'string') {
return attributes.id;
}
Expand All @@ -21,11 +30,11 @@ export const heading: Schema = {
id: { type: String },
level: { type: Number, required: true, default: 1 },
},
transform(node, config) {
transform(node, config: ConfigTypeWithCtx) {
const { level, ...attributes } = node.transformAttributes(config);
const children = node.transformChildren(config);

const slug = getSlug(attributes, children);
const slug = getSlug(attributes, children, config.ctx.headingSlugger);

const render = config.nodes?.heading?.render ?? `h${level}`;
const tagProps =
Expand All @@ -39,3 +48,16 @@ export const heading: Schema = {
return new Markdoc.Tag(render, tagProps, children);
},
};

export function setupHeadingConfig(): ConfigTypeWithCtx {
const headingSlugger = new Slugger();

return {
ctx: {
headingSlugger,
},
nodes: {
heading,
},
};
}
2 changes: 1 addition & 1 deletion packages/integrations/markdoc/src/nodes/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { heading } from './heading.js';
export { headingSlugger } from './heading.js';
export { setupHeadingConfig } from './heading.js';

export const nodes = { heading };
46 changes: 32 additions & 14 deletions packages/integrations/markdoc/src/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,45 @@ import Markdoc, {
type RenderableTreeNode,
} from '@markdoc/markdoc';
import type { ContentEntryModule } from 'astro';
import { nodes as astroNodes } from './nodes/index.js';
import { setupHeadingConfig } from './nodes/index.js';

/** Used to reset Slugger cache on each build at runtime */
/** Used to call `Markdoc.transform()` and `Markdoc.Ast` in runtime modules */
export { default as Markdoc } from '@markdoc/markdoc';
export { headingSlugger } from './nodes/index.js';

export function applyDefaultConfig(
config: MarkdocConfig,
entry: ContentEntryModule
): MarkdocConfig {
/**
* Merge user config with default config and set up context (ex. heading ID slugger)
* Called on each file's individual transform
*/
export function setupConfig(userConfig: MarkdocConfig, entry: ContentEntryModule): MarkdocConfig {
const defaultConfig: MarkdocConfig = {
// `setupXConfig()` could become a "plugin" convention as well?
...setupHeadingConfig(),
variables: { entry },
};
return mergeConfig(defaultConfig, userConfig);
}

/** Merge function from `@markdoc/markdoc` internals */
function mergeConfig(configA: MarkdocConfig, configB: MarkdocConfig): MarkdocConfig {
return {
...config,
variables: {
entry,
...config.variables,
...configA,
...configB,
tags: {
...configA.tags,
...configB.tags,
},
nodes: {
...astroNodes,
...config.nodes,
...configA.nodes,
...configB.nodes,
},
functions: {
...configA.functions,
...configB.functions,
},
variables: {
...configA.variables,
...configB.variables,
},
// TODO: Syntax highlighting
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Our heading ID generator can have a stale cache for duplicates. Let's check for those!

# Level 1 heading

## Level **2 heading**

### Level _3 heading_

#### Level [4 heading](/with-a-link)

##### Level 5 heading with override {% #id-override %}

###### Level 6 heading
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
---
import { getEntryBySlug } from "astro:content";
import { getCollection, CollectionEntry } from "astro:content";
const post = await getEntryBySlug('docs', 'headings');
const { Content, headings } = await post.render();
export async function getStaticPaths() {
const docs = await getCollection('docs');
return docs.map(doc => ({ params: { slug: doc.slug }, props: doc }));
}
type Props = CollectionEntry<'docs'>;
const { Content, headings } = await Astro.props.render();
---

<!DOCTYPE html>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Our heading ID generator can have a stale cache for duplicates. Let's check for those!

# Level 1 heading

## Level **2 heading**

### Level _3 heading_

#### Level [4 heading](/with-a-link)

##### Level 5 heading with override {% #id-override %}

###### Level 6 heading
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
---
import { getEntryBySlug } from "astro:content";
import { getCollection, CollectionEntry } from "astro:content";
const post = await getEntryBySlug('docs', 'headings');
const { Content, headings } = await post.render();
export async function getStaticPaths() {
const docs = await getCollection('docs');
return docs.map(doc => ({ params: { slug: doc.slug }, props: doc }));
}
type Props = CollectionEntry<'docs'>;
const { Content, headings } = await Astro.props.render();
---

<!DOCTYPE html>
Expand Down
50 changes: 40 additions & 10 deletions packages/integrations/markdoc/test/headings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,23 @@ describe('Markdoc - Headings', () => {
});

it('applies IDs to headings', async () => {
const res = await fixture.fetch('/');
const res = await fixture.fetch('/headings');
const html = await res.text();
const { document } = parseHTML(html);

idTest(document);
});

it('generates the same IDs for other documents with the same headings', async () => {
const res = await fixture.fetch('/headings-stale-cache-check');
const html = await res.text();
const { document } = parseHTML(html);

idTest(document);
});

it('generates a TOC with correct info', async () => {
const res = await fixture.fetch('/');
const res = await fixture.fetch('/headings');
const html = await res.text();
const { document } = parseHTML(html);

Expand All @@ -49,14 +57,21 @@ describe('Markdoc - Headings', () => {
});

it('applies IDs to headings', async () => {
const html = await fixture.readFile('/index.html');
const html = await fixture.readFile('/headings/index.html');
const { document } = parseHTML(html);

idTest(document);
});

it('generates the same IDs for other documents with the same headings', async () => {
const html = await fixture.readFile('/headings-stale-cache-check/index.html');
const { document } = parseHTML(html);

idTest(document);
});

it('generates a TOC with correct info', async () => {
const html = await fixture.readFile('/index.html');
const html = await fixture.readFile('/headings/index.html');
const { document } = parseHTML(html);

tocTest(document);
Expand All @@ -83,23 +98,31 @@ describe('Markdoc - Headings with custom Astro renderer', () => {
});

it('applies IDs to headings', async () => {
const res = await fixture.fetch('/');
const res = await fixture.fetch('/headings');
const html = await res.text();
const { document } = parseHTML(html);

idTest(document);
});

it('generates the same IDs for other documents with the same headings', async () => {
const res = await fixture.fetch('/headings-stale-cache-check');
const html = await res.text();
const { document } = parseHTML(html);

idTest(document);
});

it('generates a TOC with correct info', async () => {
const res = await fixture.fetch('/');
const res = await fixture.fetch('/headings');
const html = await res.text();
const { document } = parseHTML(html);

tocTest(document);
});

it('renders Astro component for each heading', async () => {
const res = await fixture.fetch('/');
const res = await fixture.fetch('/headings');
const html = await res.text();
const { document } = parseHTML(html);

Expand All @@ -113,21 +136,28 @@ describe('Markdoc - Headings with custom Astro renderer', () => {
});

it('applies IDs to headings', async () => {
const html = await fixture.readFile('/index.html');
const html = await fixture.readFile('/headings/index.html');
const { document } = parseHTML(html);

idTest(document);
});

it('generates the same IDs for other documents with the same headings', async () => {
const html = await fixture.readFile('/headings-stale-cache-check/index.html');
const { document } = parseHTML(html);

idTest(document);
});

it('generates a TOC with correct info', async () => {
const html = await fixture.readFile('/index.html');
const html = await fixture.readFile('/headings/index.html');
const { document } = parseHTML(html);

tocTest(document);
});

it('renders Astro component for each heading', async () => {
const html = await fixture.readFile('/index.html');
const html = await fixture.readFile('/headings/index.html');
const { document } = parseHTML(html);

astroComponentTest(document);
Expand Down

0 comments on commit a9e1cd7

Please sign in to comment.