Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backslash sometimes inserted when using $ in text #606

Closed
lourd opened this issue Jun 18, 2019 · 15 comments · Fixed by #740 or #1382
Closed

Backslash sometimes inserted when using $ in text #606

lourd opened this issue Jun 18, 2019 · 15 comments · Fixed by #740 or #1382
Labels
🐛 type/bug This is a problem

Comments

@lourd
Copy link

lourd commented Jun 18, 2019

I noticed that on some parts of my portfolio site (made with gatsby and gatsby-mdx), there are extra backslashes being inserted in some places where dollar signs are used. For example:

  • Go to https://descioli.design/grove and Find for \$ — there are 6 occurrences on the page. There are 12 usages of $, though — not every $ is preceded by a backslash.
  • The MDX source for that page is here. Note: no backslashes.
  • The rendering of the page is here. Pretty standard AFAIK.

It only started happening when I switched from Remark to MDX. I didn't notice it when I did the initial switch.

Environment

Steps to reproduce

Clone my portfolio repo, https://github.com/lourd/descioli-design, and run it. No additional configuration needed.

Expected behaviour

No extra backslashes.

Actual behaviour

Extra backslashes

@lourd
Copy link
Author

lourd commented Jun 21, 2019

This ended up being due to prettier. I'd set it up to run on pre-commit and missed the fact that it was also formatting .mdx files 🤦‍♂ Sorry to waste your time.

@lourd lourd closed this as completed Jun 21, 2019
@johno
Copy link
Member

johno commented Jun 21, 2019

I'm going to reopen since MDX output is in fact different than remark's. It's a bug in the template literal escaping that we'll need to address.

@johno johno reopened this Jun 21, 2019
@lourd
Copy link
Author

lourd commented Jun 24, 2019 via email

@millette
Copy link
Contributor

Should mdx handle $ and remove the \ when output, since $ must be escaped due to math support - see prettier/prettier#6213.

@johno
Copy link
Member

johno commented Aug 19, 2019

Yeah, ultimately we should detect the presence of \$ and not escape it twice.

In the ideal world Prettier wouldn't add the $ escaping when there isn't inline math being used (or it at least wasn't the default), but it seems like we've reached the point where we need to solve it on our end.

@millette
Copy link
Contributor

I even tried something like cost: 5{"$"} but that was output litteraly.

johno added a commit that referenced this issue Aug 20, 2019
Prettier escapes all "$" characters that aren't used for
inline math. This ensures MDX doesn't double escape the
"\$", otherwise "\$" ends up rendered in the document.

Closes #606
@johno johno closed this as completed in #740 Sep 4, 2019
johno added a commit that referenced this issue Sep 4, 2019
* Don't double escape "$" characters that are alread escaped

Prettier escapes all "$" characters that aren't used for
inline math. This ensures MDX doesn't double escape the
"\$", otherwise "\$" ends up rendered in the document.

Closes #606

* Use proper negative lookahead regex

* Add more tests around dollar escaping

* Make eslint happy
@adamwathan
Copy link

Sorry to comment on a closed issue but I'm still seeing MDX render \$ in the HTML if the dollar sign is escaped in the MDX. It's a Next.js project, and here are my dependencies:

{
  "name": "tailwind-jobs",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start"
  },
  "dependencies": {
    "@mdx-js/loader": "^1.6.1",
    "@next/mdx": "^9.4.0",
    "@tailwindcss/ui": "^0.3.0",
    "autoprefixer": "^9.7.6",
    "next": "9.4.0",
    "postcss-nesting": "^7.0.1",
    "react": "16.13.1",
    "react-dom": "16.13.1",
    "tailwindcss": "^1.4.6"
  }
}

Is there a dependency I need to explicitly add that I'm missing, to make sure I'm actually grabbing a recent enough version to get this fix?

@johno johno reopened this May 13, 2020
@flikteoh
Copy link

flikteoh commented Jul 14, 2020

Hey @johno

Is this still being worked on?

Still having the \$ issue with Gatsby + MDX

@NathanHealea
Copy link

NathanHealea commented Aug 15, 2020

@flikteoh and @johno,

I'am also seeing the same issue \$10 when using Gatsby + MDX.

@emanuelturis
Copy link

Putting the text inside a p tag has prevented Prettier from adding a the \ before the $ when formatting.

<p>
  The price is $30/month.
</p>

@samajammin
Copy link

Hey @lourd - I'm having the same issue (pre-commit hook w/ Prettier). How did you prevent Prettier from doing this?

Thanks in advance!

wooorm added a commit that referenced this issue Dec 15, 2020
This PR changes the internals of the core `@mdx-js/mdx` package to generate a
JavaScript syntax tree instead of a string.
This fixes escaping issues such as #1219.
It makes `mdx-hast-to-jsx` much more palatable.
It also prevents several Babel parses.
It paves the way for passing in Babel plugins, which is useful for users, but
also for us to compile to `React.createElement`, `_jsx`, or Vue’s `h` calls
directly and make MDX’s output directly usable.

* `babel-plugin-apply-mdx-type-props`: add `parentType`
* `mdx`: use `rehype-minify-whitespace` to remove superfluous whitespace
* `mdx`: use `hast-util-to-estree` to transform hast to estree
* `mdx`: use `estree-to-babel` to transform estree to Babel
* `mdx`: generate estree/Babel instead of strings
* `mdx`: use `@babel/generator` to serialize Babel AST
* `vue`: stop supporting the react transform: (it doesn’t make sense)
* `vue`: fix support for props to components

Related to GH-741.
Related to GH-1152.

Closes GH-606.
Closes GH-1028.
Closes GH-1219.
wooorm added a commit that referenced this issue Dec 18, 2020
This PR changes the internals of the core `@mdx-js/mdx` package to generate a
JavaScript syntax tree instead of a string.
This fixes escaping issues such as #1219.
It makes `mdx-hast-to-jsx` much more palatable.
It also prevents several Babel parses.
It paves the way for passing in Babel plugins, which is useful for users, but
also for us to compile to `React.createElement`, `_jsx`, or Vue’s `h` calls
directly and make MDX’s output directly usable.

* `babel-plugin-apply-mdx-type-props`: add `parentType`
* `mdx`: use `rehype-minify-whitespace` to remove superfluous whitespace
* `mdx`: use `hast-util-to-estree` to transform hast to estree
* `mdx`: use `estree-to-babel` to transform estree to Babel
* `mdx`: generate estree/Babel instead of strings
* `mdx`: use `@babel/generator` to serialize Babel AST
* `vue`: stop supporting the react transform: (it doesn’t make sense)
* `vue`: fix support for props to components

Related to GH-741.
Related to GH-1152.

Closes GH-606.
Closes GH-1028.
Closes GH-1219.
wooorm added a commit that referenced this issue Dec 18, 2020
This PR changes the internals of the core `@mdx-js/mdx` package to generate a
JavaScript syntax tree instead of a string.
This fixes escaping issues such as #1219.
It makes `mdx-hast-to-jsx` much more palatable.
It also prevents several Babel parses.
It paves the way for passing in Babel plugins, which is useful for users, but
also for us to compile to `React.createElement`, `_jsx`, or Vue’s `h` calls
directly and make MDX’s output directly usable.

* `babel-plugin-apply-mdx-type-props`: add `parentType`
* `mdx`: use `rehype-minify-whitespace` to remove superfluous whitespace
* `mdx`: use `hast-util-to-estree` to transform hast to estree
* `mdx`: use `estree-to-babel` to transform estree to Babel
* `mdx`: generate estree/Babel instead of strings
* `mdx`: use `@babel/generator` to serialize Babel AST
* `vue`: stop supporting the react transform: (it doesn’t make sense)
* `vue`: fix support for props to components

Related to GH-741.
Related to GH-1152.

Closes GH-606.
Closes GH-1028.
Closes GH-1219.
Closes GH-1382.

Reviewed-by: Christian Murphy <[email protected]>
wooorm added a commit that referenced this issue Dec 18, 2020
This PR changes the internals of the core `@mdx-js/mdx` package to generate a
JavaScript syntax tree instead of a string.
This fixes escaping issues such as #1219.
It makes `mdx-hast-to-jsx` much more palatable.
It also prevents several Babel parses.
It paves the way for passing in Babel plugins, which is useful for users, but
also for us to compile to `React.createElement`, `_jsx`, or Vue’s `h` calls
directly and make MDX’s output directly usable.

* `babel-plugin-apply-mdx-type-props`: add `parentType`
* `mdx`: use `rehype-minify-whitespace` to remove superfluous whitespace
* `mdx`: use `hast-util-to-estree` to transform hast to estree
* `mdx`: use `estree-to-babel` to transform estree to Babel
* `mdx`: generate estree/Babel instead of strings
* `mdx`: use `@babel/generator` to serialize Babel AST
* `vue`: stop supporting the react transform: (it doesn’t make sense)
* `vue`: fix support for props to components

Related to GH-741.
Related to GH-1152.

Closes GH-606.
Closes GH-1028.
Closes GH-1219.
Closes GH-1382.

Reviewed-by: Christian Murphy <[email protected]>
jgosmann added a commit to jgosmann/adventures that referenced this issue Jan 5, 2021
As prettier has finally been fixed to not re-add it.

See:
* prettier/prettier#6213
* mdx-js/mdx#606
@flores8
Copy link

flores8 commented Apr 7, 2021

If anyone else runs into this problem like I did today, here's how I fixed it. I needed prettier to stop adding in the backslash. I first started looking in the prettier settings to see if I could turn off the math portion but that ended up being a dead end. I realized that I needed to tell prettier to simply ignore it.

I first tried to my prettier ignore file: https://media.lauraleeflores.com/llu5D48j that works however it will ignore the page completely and that was not ideal for me.

The solution that worked was to simply ignore a portion of the file. You can learn more about that here: https://prettier.io/docs/en/ignore.html

Mine is an mdx file so this is how I ignored it: https://media.lauraleeflores.com/YEuR1dYj

I hope that helps someone who runs into the same problem.

@sebastienbarre
Copy link

sebastienbarre commented Mar 7, 2022

Is there any plan on fixing this for * as well?

For example, in a MDX file:

MDX also supports JavaScript expressions inside curly braces, i.e.: {Math.PI * 2}

Prettier automatically changes it to:

MDX also supports JavaScript expressions inside curly braces, i.e.: {Math.PI \* 2}

Which breaks MDX parsers -- this is no longer a valid Javascript/JSX expression.

Using Prettier's recommended <!-- prettier-ignore --> won't help because it breaks the MDX parser as well (because of the < used for React/JSX components), and using MDX's recommend {/* this is a comment */} doesn't work because it is not recognized by Prettier since the file is treated as Markdown (as it should), not JSX.

Thanks

@ChristianMurphy
Copy link
Member

The * escaping is due to prettier currently supporting only MDX 1 rather than MDX 2, meaning it currently doesn't recognize the inline JSX expression and adds an escape.
MDX 2 support in prettier is being discussed at prettier/prettier#12209

@wooorm
Copy link
Member

wooorm commented Mar 7, 2022

To add (or explicitly clarify): The JS in braces are a new MDX 2 thing. So, we just added support for them. Now other tools have to support them too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 type/bug This is a problem