fix: allow hyphenated variables in MDX by HugoHSun · Pull Request #1320 · readmeio/markdown

HugoHSun · 2026-02-03T04:55:14Z

	Fix CX-2880

🧰 Changes

Provides a workaround for user variables with hyphen in the MDX Slate editor, originially - always get parsed to the subtraction operator in JS.

In the rare case of using the actual subtraction between an user variable and a defined constant, it would need to have space around the operator e.g.
export const total = 10
{user.variable - total}
or else {user.variable-total} get interpreted as a custom variable variable-total

Editor

View

🧬 QA & Testing

HugoHSun · 2026-02-03T05:21:01Z

processor/transform/variables.ts

+  if (expr.type !== 'BinaryExpression' || expr.operator !== '-') return false;
+
+  // Check right side is an Identifier
+  if (expr.right.type !== 'Identifier') return false;


To prevent edge cases like {user.number_variable - 1} or {user.num-1} being parsed as hyphenated variable.

can you add a test for this and/or other edge cases? i can imagine there are a few others, and if we're only verifying the "right" side to not be an identifier, is this enough?

thinking of some others:

if we have something like {1 - user.some-variable} or {1 - user.variable}, what's the correct behavior that should happen here?

what about `{user.number_var - 1 - user.another_var}? this check would fail and you may proceed to think this is a hyphenated user var right?

this raises some good points

for {1 - user.some-variable} or {1 - user.variable}, it's will pass here but not transformed to as it fails the regex check later requiring to start with {user.

{user.number_var - 1 - user.another_var} won't transform too
BinaryExpression ← expr (outermost)
├── left: BinaryExpression
│ ├── left: MemberExpression(user.number_var)
│ ├── operator: '-'
│ └── right: Literal(1)
├── operator: '-'
└── right: MemberExpression(user.another_var) ← expr.right

rafegoldberg

Logic looks fine, but this code needs clean up.

rafegoldberg · 2026-02-04T17:35:41Z

processor/transform/variables.ts

+  // Check right side is an Identifier
+  if (expr.right.type !== 'Identifier') return false;


This comment is redundant (and also says the opposite of what the next line checks for, which is confusing.) Please remove:

Suggested change

// Check right side is an Identifier

if (expr.right.type !== 'Identifier') return false;

if (expr.right.type !== 'Identifier') return false;

rafegoldberg · 2026-02-04T17:37:46Z

processor/transform/variables.ts

+  // Must be a BinaryExpression with subtraction operator
+  if (expr.type !== 'BinaryExpression' || expr.operator !== '-') return false;


This comment is redundant (and also says the opposite of what the next line checks for, which is confusing.) Please remove:

Suggested change

// Must be a BinaryExpression with subtraction operator

if (expr.type !== 'BinaryExpression' || expr.operator !== '-') return false;

if (expr.type !== 'BinaryExpression' || expr.operator !== '-') return false;

rafegoldberg · 2026-02-04T17:44:08Z

processor/transform/variables.ts

+        // Hyphenated variable: `user.X-API-Key` parsed as `user.X - API - Key`
+        // Extract the full variable name from node.value
+        const value = node.value;
+        const match = value.match(/^user\.([a-zA-Z_][a-zA-Z0-9_-]*)$/);


This regex is unnecessarily complicated. Let's use the built in alphanumeric character class to simplify this:

Suggested change

const match = value.match(/^user\.([a-zA-Z_][a-zA-Z0-9_-]*)$/);

const match = value.match(/^user\.([\w-]*)$/);

dannobytes · 2026-02-04T22:47:27Z

processor/transform/variables.ts

+  if (expr.type !== 'BinaryExpression' || expr.operator !== '-') return false;
+
+  // Check right side is an Identifier
+  if (expr.right.type !== 'Identifier') return false;


can you add a test for this and/or other edge cases? i can imagine there are a few others, and if we're only verifying the "right" side to not be an identifier, is this enough?

thinking of some others:

if we have something like {1 - user.some-variable} or {1 - user.variable}, what's the correct behavior that should happen here?

what about `{user.number_var - 1 - user.another_var}? this check would fail and you may proceed to think this is a hyphenated user var right?

dannobytes · 2026-02-04T22:50:12Z

__tests__/transformers/variables.test.ts

+    expect(code.type).toBe('code');
+    expect(code.value).toBe('{user.X-API-Key}');
+  });
+});


can we add a few more tests for other edge cases where the transformer should not parse the raw md as a readme variable?

hmmm I think this is more tricky than it seems since the current behaviour (without this PR) is technically correct for JS expressions in MDX.

the PR logic transforms {user.a-1-b} as variable a-1-b, but {user.a-b-1} is still the expression user.a - b - 1

{user.a-false} is user.a - false since false is a literal not identifier

hyphenated variables won't be transformed if it's not at the start of the expression. So {1 - user.some-variable} is still evaluated as the expression {1 - user.some - variable}

I think it's fine to only allow hyphenated variables at the start, but should we allow non-identifiers (e.g. const, true, false, 1...) at the end of the variable?

another concern would be that it's gonna impact the customers who actually want to do subtraction rather than variable names. e.g.

rafegoldberg

QAed this and it doesn't work at all. Try it for yourself by running the start script on this branch, adding a hyphenated my-var here, and then hitting this link:

Also, per @kevinports' comment here the current approach is to use proper bracket accessor notation in these instances, so: {user['my-var']}. Not sure we actually want to allow random hyphenated values within this expression syntax in the first place?

rafegoldberg · 2026-02-12T17:21:38Z

processor/transform/variables.ts

+        // Hyphenated variable: `user.X-API-Key` parsed as `user.X - API - Key`
+        // Extract the full variable name from node.value
+        const value = node.value;
+        const match = value.match(/^user\.([\w-]*)$/);


Per this thread with @maximilianfalco, I was wrong about the \w selector. Let's revert back to your original @HugoHSun.

Sorry I should have made the context more clear:

This PR is targeted at MDX and variablesTransformer only runs in mdast() (via ast-processor.ts), which is NOT used by compile(). So here it always get compiled as the JS expression.

The only path where this PR's fix actually works is the visual editor in the main app:

Slate's normalizeNode calls renderingLibrary.mdast() → variablesTransformer detects the
hyphenated variable → creates a readme-variable node

On save, readme-to-mdx.ts serializes it as {user["x-api"]} because x-api isn't a valid JS identifier

The viewer's compile() → run() handles bracket notation fine

Screen.Recording.2026-02-13.at.5.07.55.pm.mov

After thinking about it more, I reckon too we shouldn't allow hyphenated variables in dot notation (e.g. {user.test-var}). Especially in this package since it's also used outside of the app. It introduces ambiguity on whether it's an hyphenated variable or minus operation. Bracket notation for weird variable names (e.g. {user["test-var"], {user["test var"]}) is the way to go since they are valid JS expressions which get evaluated with the user proxy.

rafegoldberg

whoops, did not mean to approve this as it isn't working as stated rn. see above for detail.

HugoHSun · 2026-02-13T10:40:25Z

Update: closing this PR since the fix for bracket variables are in https://github.com/readmeio/readme/pull/17315

fix: allow hyphenated variables in MDX

48db47e

HugoHSun commented Feb 3, 2026

View reviewed changes

HugoHSun marked this pull request as ready for review February 3, 2026 06:02

HugoHSun requested review from domharrington and julshotal February 3, 2026 10:58

erunion requested review from dannobytes, kevinports and rafegoldberg and removed request for domharrington and julshotal February 4, 2026 17:34

rafegoldberg requested changes Feb 4, 2026

View reviewed changes

dannobytes reviewed Feb 4, 2026

View reviewed changes

HugoHSun added 2 commits February 5, 2026 14:06

fix: code style

56c7998

test: add more testcases

ae9f139

rafegoldberg approved these changes Feb 12, 2026

View reviewed changes

rafegoldberg requested changes Feb 12, 2026

View reviewed changes

HugoHSun mentioned this pull request Feb 13, 2026

fix: stabilies heading slug generations #1334

Closed

HugoHSun closed this Feb 13, 2026

		// Check right side is an Identifier
		if (expr.right.type !== 'Identifier') return false;

		// Must be a BinaryExpression with subtraction operator
		if (expr.type !== 'BinaryExpression' \|\| expr.operator !== '-') return false;

	const match = value.match(/^user\.([a-zA-Z_][a-zA-Z0-9_-]*)$/);
	const match = value.match(/^user\.([\w-]*)$/);

Conversation

HugoHSun commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧰 Changes

Editor

View

🧬 QA & Testing

Uh oh!

HugoHSun Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannobytes Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

HugoHSun Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

rafegoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

rafegoldberg Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafegoldberg Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafegoldberg Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannobytes Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

dannobytes Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

HugoHSun Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HugoHSun Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

rafegoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

rafegoldberg Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

HugoHSun Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HugoHSun Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafegoldberg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HugoHSun commented Feb 13, 2026

Update: closing this PR since the fix for bracket variables are in https://github.com/readmeio/readme/pull/17315

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HugoHSun commented Feb 3, 2026 •

edited

Loading

HugoHSun Feb 3, 2026 •

edited

Loading

rafegoldberg Feb 4, 2026 •

edited

Loading

rafegoldberg Feb 4, 2026 •

edited

Loading

rafegoldberg Feb 4, 2026 •

edited

Loading

HugoHSun Feb 5, 2026 •

edited

Loading

HugoHSun Feb 13, 2026 •

edited

Loading

HugoHSun Feb 13, 2026 •

edited

Loading

rafegoldberg left a comment •

edited

Loading