Skip to content

Conversation

@morgs32
Copy link

@morgs32 morgs32 commented Oct 2, 2018

React throws a warning:
Warning: React does not recognize the metaString prop on a DOM element.

pasted_image_10_2_18__1_22_pm

React throws a warning:
`Warning: React does not recognize the `metaString` prop on a DOM element.`
@vercel
Copy link

vercel bot commented Oct 2, 2018

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@silvenon
Copy link
Contributor

silvenon commented Oct 2, 2018

The infostring is added on purpose, but maybe @ChristopherBiscardi can shed some light on whether the metaString prop itself is necessary, because the following:

```js repl id=123
const a = 5
```

passes these props to the code block:

{
  metaString: "repl id=123",
  repl: true,
  id: 123,
}

In my opinion the metaString prop isn't useful, but maybe it has its reasons. In any case, if you're using infostrings it's expected that you're somehow making use of those props and it's on you to not pass it to intrinsic elements. However, even not using an infostring still seems to pass metaString: "", so that should probably be removed.

@ChristopherBiscardi
Copy link
Member

It's there in case someone needs the raw output after the language was stripped off. For example, the current implementation for props doesn't support arrays and you could implement support in userland by parsing the metaString prop differently.

The empty-string metaString being passed through is definitely a bug and can be removed (but that's not what this PR does). Something like this would work:

const metaString = node.lang && node.lang.replace(langRegex, '').trim()
props.metaString = metaString !== "" && metaString

@silvenon
Copy link
Contributor

silvenon commented Oct 3, 2018

@morgs32 are you interested in modifying this PR to remove metaString if it's an empty string, accompanied by a test proving this?

@morgs32
Copy link
Author

morgs32 commented Oct 3, 2018

@silvenon happy to. Expect updates and test by EOD.

@silvenon silvenon changed the title Update mdx-ast-to-mdx-hast.js Don't include metaString in codee props Nov 3, 2018
@silvenon silvenon changed the title Don't include metaString in codee props Don't include metaString in code props Nov 3, 2018
@silvenon silvenon closed this in ca5ba71 Nov 3, 2018
@morgs32 morgs32 deleted the patch-1 branch November 5, 2018 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants