Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions __tests__/lib/mdxish/variables-code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,36 @@ const name = 'Bearer ${variable}';
expect(getCodeText(tree)).toBe('sk_live_123');
});

it('stringifies structured variables in code', () => {
const tree = mdxish('`<<keys>> {user.profile} {user.limit} {user.active}`', {
variables: {
user: {
active: true,
keys: [{ apiKey: 'rdme_123' }],
limit: 25,
profile: { plan: 'enterprise' },
},
defaults: [],
},
});

expect(getCodeText(tree)).toBe('[{"apiKey":"rdme_123"}] {"plan":"enterprise"} 25 true');
});

it('coerces null and undefined user variable values to empty strings', () => {
const tree = mdxish('`<<nullValue>>|<<undefinedValue>>`', {
variables: {
user: {
nullValue: null,
undefinedValue: undefined,
},
defaults: [],
},
});

expect(getCodeText(tree)).toBe('|');
});

it('does not double-resolve when a legacy variable value contains an MDX variable pattern', () => {
const tree = mdxish('`<<payload>>`', {
variables: {
Expand Down
35 changes: 33 additions & 2 deletions __tests__/lib/render-mdxish/Variables.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,41 @@
import '@testing-library/jest-dom';
import { render } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import React from 'react';

import { mdxish, renderMdxish } from '../../../lib';

describe('render mdxish variables in code', () => {
describe('render mdxish variables', () => {
it.each([
{
expected: '[{"apiKey":"rdme_123"}]',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

mdxish resolves {user.*} as whole variable nodes rather than arbitrary JS expressions, so these cases assert the stringified value instead of drilling into nested properties like mdx.

md: '{user.keys}',
name: 'arrays',
user: { keys: [{ apiKey: 'rdme_123' }] },
},
{
expected: '{"plan":"enterprise"}',
md: '{user.profile}',
name: 'objects',
user: { profile: { plan: 'enterprise' } },
},
{
expected: '25',
md: '{user.limit}',
name: 'primitives',
user: { limit: 25 },
},
])('supports structured user variables: $name', ({ expected, md, user }) => {
const variables = {
user,
defaults: [],
};
const mod = renderMdxish(mdxish(md, { variables }), { variables });

render(<mod.default />);

expect(screen.getByText(expected)).toBeVisible();
});

it('resolves legacy and mdx variables in inline code', () => {
const md = 'Use `<<apiKey>>` and `{user.region}`';
const variables = {
Expand Down
22 changes: 22 additions & 0 deletions __tests__/lib/render-mdxish/toc.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,28 @@ describe('toc transformer', () => {
expect(screen.findByText('Setup for admins')).toBeDefined();
});

it('stringifies structured variables in labels', async () => {
const md = `# Keys {user.keys}

## Profile {user.profile} {user.limit}
`;
const variables = {
user: {
keys: [{ apiKey: 'rdme_123' }],
limit: 25,
profile: { plan: 'enterprise' },
},
defaults: [],
};

const { Toc } = renderMdxish(mdxish(md), { variables });

render(<Toc />);

expect(await screen.findByText('Keys [{"apiKey":"rdme_123"}]')).toBeDefined();
expect(await screen.findByText('Profile {"plan":"enterprise"} 25')).toBeDefined();
});

it('keeps adjacent legacy variable values and suffixes together', () => {
const md = '## Hello <<name>>! Nice';
const variables = {
Expand Down
21 changes: 21 additions & 0 deletions __tests__/plugins/toc.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,27 @@ export const toc = [
expect(screen.findByText('Setup for admins')).toBeDefined();
});

it('stringifies structured variables in labels', async () => {
const md = `# Keys {user.keys}

## Profile {user.profile} {user.limit}`;
const variables = {
user: {
keys: [{ apiKey: 'rdme_123' }],
limit: 25,
profile: { plan: 'enterprise' },
},
defaults: [],
};

const { Toc } = run(compile(md), { variables });

render(<Toc />);

expect(await screen.findByText('Keys [{"apiKey":"rdme_123"}]')).toBeDefined();
expect(await screen.findByText('Profile {"plan":"enterprise"} 25')).toBeDefined();
});

it('keeps mixed inline phrasing together', () => {
const md = '## Hello {user.name}! N*ic*e [day](https://example.com)s';
const variables = {
Expand Down
27 changes: 27 additions & 0 deletions __tests__/variables/index.test.tsx
Comment thread
RAYMOND-LUO marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,31 @@ export const Hello = () => <p>{user.name}</p>;

expect(screen.getByText('Owlbert')).toBeVisible();
});

Comment thread
RAYMOND-LUO marked this conversation as resolved.
it.each([
{
expected: 'rdme_123',
md: '{user.keys[0].apiKey}',
name: 'arrays',
user: { keys: [{ apiKey: 'rdme_123' }] },
},
{
expected: 'enterprise',
md: '{user.profile.plan}',
name: 'objects',
user: { profile: { plan: 'enterprise' } },
},
{
expected: 'active 25',
md: "{user.active ? 'active' : 'inactive'} {user.limit}",
name: 'primitives',
user: { active: true, limit: 25 },
},
])('supports structured user variables: $name', ({ expected, md, user }) => {
const Content = execute(md, {}, { variables: { user } });

render(<Content />);

expect(screen.getByText(expected)).toBeVisible();
});
});
3 changes: 2 additions & 1 deletion processor/plugin/toc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { h } from 'hastscript';

import { mdx, plain } from '../../lib';
import { STANDARD_HTML_TAGS } from '../../utils/common-html-words';
import { flattenUserVariables } from '../../utils/user';
import { hasNamedExport } from '../utils';

const HEADING_TAGS = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'];
Expand Down Expand Up @@ -149,7 +150,7 @@ const getDepth = (el: HastHeading) => {
const flattenVariables = (variables?: Variables): Record<string, string> => {
if (!variables) return {};
return {
...variables.user,
...flattenUserVariables(variables.user),
...Object.fromEntries(
(variables.defaults || []).filter(d => !(d.name in variables.user)).map(d => [d.name, d.default]),
),
Expand Down
4 changes: 3 additions & 1 deletion processor/transform/mdxish/variables-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import type { Plugin } from 'unified';
import { MDX_VARIABLE_REGEXP, VARIABLE_REGEXP } from '@readme/variable';
import { visit } from 'unist-util-visit';

import { flattenUserVariables } from '../../../utils/user';

interface Options {
variables?: Variables;
}
Expand All @@ -18,7 +20,7 @@ function flattenVariables(variables?: Variables): Record<string, string> {

return {
...Object.fromEntries((variables.defaults || []).map(d => [d.name, d.default])),
...variables.user,
...flattenUserVariables(variables.user),
};
}

Expand Down
2 changes: 1 addition & 1 deletion types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ interface TocList extends Element {

interface Variables {
defaults: { default: string; name: string }[];
user: Record<string, string>;
user: Record<string, unknown>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change makes sense but just be careful since this type gets used in several places including the main readme repo 🙏 Hopefully it doesn't break

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea good point - funny enough it looks like the readme repo imports Variables from packages/iso/src/types/rdmd.ts and not directly from this package, and that type already declares user: Record<string, unknown>. So widening here brings @readme/markdown in line with what readme repo already expects.

At least to my understanding, the only direct consumer that needed updating is mdx-renderer (handled in my other pr)

}

interface TocListItem extends Element {
Expand Down
23 changes: 22 additions & 1 deletion utils/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,30 @@ interface Default {

export interface Variables {
defaults: Default[];
user: Record<string, string>;
user: Record<string, unknown>;
}

/**
* Coerce a user variable value to a string for substitution into markdown text.
* Non-string values (arrays, objects, numbers) are stringified via JSON or `String()`
* so that `<<var>>` syntax doesn't produce `[object Object]` for structured data like
* JWT `keys`.
*/
const stringifyVariableValue = (value: unknown): string => {
if (typeof value === 'string') return value;
if (value == null) return '';
if (typeof value === 'object') return JSON.stringify(value) ?? '';
return String(value);
};

/**
* Flatten `variables.user` into a string-keyed string-valued record by coercing
* each value. Used by markdown substitution paths that need a plain
* `Record<string, string>` lookup.
*/
export const flattenUserVariables = (user: Record<string, unknown>): Record<string, string> =>
Object.fromEntries(Object.entries(user).map(([name, value]) => [name, stringifyVariableValue(value)]));

const User = (variables?: Variables) => {
const { user = {}, defaults = [] } = variables || {};

Expand Down