Skip to content

Commit

Permalink
fix: allow spaces in variables (#1035)
Browse files Browse the repository at this point in the history
[![PR App][icn]][demo] | Fix RM-XYZ
:-------------------:|:----------:

## 🧰 Changes

Allows spaces in user variables.

Previous, the transformer was validating user variables with a regex
that didn't allow spaces. This PR changes the regex to allow spaces.
This also allows spaces in the variables names, which would have crashed
the parser before.

All of the following are now valid:
```
{user.name}

{ user.name }

{user['dont do this']}
```

## 🧬 QA & Testing

- [Broken on production][prod].
- [Working in this PR app][demo].


[demo]: https://markdown-pr-PR_NUMBER.herokuapp.com
[prod]: https://SUBDOMAIN.readme.io
[icn]:
https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg
  • Loading branch information
kellyjosephprice authored Dec 5, 2024
1 parent 47aca89 commit 5733446
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 6 deletions.
9 changes: 8 additions & 1 deletion __tests__/compilers/variable.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as rmdx from '../../index';

describe('variable compiler', () => {
it('compiles back to the ', () => {
it('compiles back to the original mdx', () => {
const mdx = `
## Hello!
Expand All @@ -13,4 +13,11 @@ describe('variable compiler', () => {

expect(rmdx.mdx(tree).trim()).toStrictEqual(mdx.trim());
});

it('with spaces in a variable, it compiles back to the original', () => {
const mdx = `{user["oh no"]}`;
const tree = rmdx.mdast(mdx);

expect(rmdx.mdx(tree).trim()).toStrictEqual(mdx.trim());
});
});
8 changes: 8 additions & 0 deletions __tests__/lib/mdast/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import tablesJson from './tables/out.json';
import variablesMdx from './variables/in.mdx?raw';
import variablesJson from './variables/out.json';

import variablesWithSpacesMdx from './variables-with-spaces/in.mdx?raw';
import variablesWithSpacesJson from './variables-with-spaces/out.json';

import inlineImagesMdx from './images/inline/in.mdx?raw';
import inlineImagesJson from './images/inline/out.json';

Expand All @@ -28,6 +31,11 @@ describe('mdast transformer', async () => {
expect(mdast(variablesMdx)).toStrictEqualExceptPosition(variablesJson);
});

it('parses variables with spaces', () => {
// @ts-ignore
expect(mdast(variablesWithSpacesMdx)).toStrictEqualExceptPosition(variablesWithSpacesJson);
});

it('parses inline images', () => {
// @ts-ignore
expect(mdast(inlineImagesMdx)).toStrictEqualExceptPosition(inlineImagesJson);
Expand Down
1 change: 1 addition & 0 deletions __tests__/lib/mdast/variables-with-spaces/in.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello, { user['this is cursed'] }
72 changes: 72 additions & 0 deletions __tests__/lib/mdast/variables-with-spaces/out.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
{
"children": [
{
"children": [
{
"position": {
"end": {
"column": 8,
"line": 1,
"offset": 7
},
"start": {
"column": 1,
"line": 1,
"offset": 0
}
},
"type": "text",
"value": "Hello, "
},
{
"data": {
"hName": "Variable",
"hProperties": {
"name": "this is cursed"
}
},
"position": {
"end": {
"column": 19,
"line": 1,
"offset": 18
},
"start": {
"column": 8,
"line": 1,
"offset": 7
}
},
"type": "readme-variable",
"value": "{ user['this is cursed'] }"
}
],
"position": {
"end": {
"column": 19,
"line": 1,
"offset": 18
},
"start": {
"column": 1,
"line": 1,
"offset": 0
}
},
"type": "paragraph"
}
],
"position": {
"end": {
"column": 1,
"line": 2,
"offset": 19
},
"start": {
"column": 1,
"line": 1,
"offset": 0
}
},
"type": "root"
}
7 changes: 7 additions & 0 deletions __tests__/transformers/variables.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,11 @@ describe('variables transformer', () => {
type: 'root',
});
});

it('does not parse regular expressions into variables', () => {
const mdx = `{notUser.name}`;

// @ts-ignore
expect(rmdx.mdast(mdx).children[0].type).toBe('mdxFlowExpression');
});
});
2 changes: 1 addition & 1 deletion processor/compile/variable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const variable = (node: Variable) => {
// @note: coming from RDMD, it's set as `variable`. But when mdx is parsed,
// it's set as `name`
const name = node.data.hProperties.variable || node.data.hProperties.name;
return `{user.${name}}`;
return name.toString().match(/ /) ? `{user[${JSON.stringify(name)}]}` : `{user.${name}}`;
};

export default variable;
22 changes: 18 additions & 4 deletions processor/transform/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,22 @@ const variables =
visit(tree, (node, index, parent) => {
if (!['mdxFlowExpression', 'mdxTextExpression'].includes(node.type) || !('value' in node)) return;

const match = node.value.match(/^user\.(?<value>.*)$/);
if (!match) return;
// @ts-expect-error - estree is not defined on our mdx types?!
if (node.data.estree.type !== 'Program') return;
// @ts-expect-error - estree is not defined on our mdx types?!
const [expression] = node.data.estree.body;
if (
!expression ||
expression.type !== 'ExpressionStatement' ||
expression.expression.object?.name !== 'user' ||
!['Literal', 'Identifier'].includes(expression.expression.property?.type)
)
return;

const name =
expression.expression.property.type === 'Identifier'
? expression.expression.property.name
: expression.expression.property.value;

let variable = asMdx
? ({
Expand All @@ -23,7 +37,7 @@ const variables =
{
type: 'mdxJsxAttribute',
name: 'name',
value: match.groups.value,
value: name,
},
],
children: [],
Expand All @@ -34,7 +48,7 @@ const variables =
data: {
hName: 'Variable',
hProperties: {
name: match.groups.value,
name,
},
},
value: `{${node.value}}`,
Expand Down

0 comments on commit 5733446

Please sign in to comment.