Skip to content

fix: support structured user variables#1488

Open
RAYMOND-LUO wants to merge 5 commits into
nextfrom
raymond/RM-16731
Open

fix: support structured user variables#1488
RAYMOND-LUO wants to merge 5 commits into
nextfrom
raymond/RM-16731

Conversation

@RAYMOND-LUO

@RAYMOND-LUO RAYMOND-LUO commented May 26, 2026

Copy link
Copy Markdown
Contributor
🎫 Resolve RM-16731

🎯 What does this PR do?

  • Updates markdown variable typing so variables.user supports structured values, not only strings.
  • This matches real authenticated/admin preview payloads where user.keys can be an array of key objects.
  • Adds explicit stringification for static variable substitution paths, so code/TOC rendering can still safely resolve non-string values.
  • This is the source package change that should ship before the mdx-renderer PR https://github.com/readmeio/mdx-renderer/pull/317, so mdx-renderer can consume the corrected @readme/markdown types. See that PR or the ticket for more context.

🧪 QA tips

📸 Screenshot or Loom

N/A, renderer/type behavior only.

@RAYMOND-LUO RAYMOND-LUO requested a review from flinehan May 26, 2026 07:56

@eaglethrost eaglethrost left a comment

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.

Have some comments on this!

Comment thread types.d.ts
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)

Comment thread __tests__/variables/index.test.tsx
Comment thread __tests__/variables/index.test.tsx
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.

@gkoberger gkoberger requested a review from a team June 7, 2026 00:21
@jboyens jboyens force-pushed the raymond/RM-16731 branch from f5be555 to 0fdeb33 Compare June 8, 2026 18:00
@jboyens jboyens force-pushed the raymond/RM-16731 branch from 2685cb9 to 0fdeb33 Compare June 11, 2026 07:41
@erunion erunion requested review from a team and removed request for a team and flinehan June 16, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants