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

jsx approved image alt text #1578

Closed
glitteringkatie opened this issue Jun 16, 2021 · 2 comments · Fixed by #1580
Closed

jsx approved image alt text #1578

glitteringkatie opened this issue Jun 16, 2021 · 2 comments · Fixed by #1580
Labels
🏡 area/internal This affects the hidden internals good first issue 👋 This may be a great place to get started! 🐛 type/bug This is a problem 💎 v2 Issues related to v2 🙆 yes/confirmed This is confirmed and ready to be worked on

Comments

@glitteringkatie
Copy link
Contributor

Subject of the issue

tl;dr quotes in image alt text break babel because they don't get appropriately turned into jsx expressions (since jsx strings don't support \" in them)

Hi! I found that if someone uses quotation marks in their image alt (![something really "cool"](https://www.humanesociety.org/sites/default/files/styles/1240x698/public/2020-07/kitten-510651.jpg)), the outputted jsx is <img src="https://www.humanesociety.org/sites/default/files/styles/1240x698/public/2020-07/kitten-510651.jpg" alt="something really \"cool\"" parentName="p" /> Which causes babel to blow up because having escaped " marks in a JSX string is not supported turns out! This comment really helped me figure that out. So! I think the solution is to make sure that if an image alt has quotes, those should appropriately be wrapped in a jsx expression {}.

Disclaimers: I think this should be on MDX since remark is agnostic to jsx. However, I noticed this wasn't an issue with the link text (Like [link text "here"](blah.com)) and that gets handled just fine (aka gets wrapped in appropriate curly braces)--unsure if that was because of something in this project or in remark.

Your environment

Steps to reproduce

  1. have an mdx file with ![something really "cool"](https://www.humanesociety.org/sites/default/files/styles/1240x698/public/2020-07/kitten-510651.jpg) in it somewhere
  2. parse that file with mdx
  3. put that parsed content through babel (transformAsync from babel core with presetReact & presetEnv)

🎉 BONUS POINTS for creating a minimal reproduction and uploading it to GitHub. This will get you the fastest support. 🎉

Expected behaviour

Babel is happy and outputs functioning code!

Actual behaviour

Babel is sad and says "SyntaxError: unknown: Expecting Unicode escape sequence \uXXXX"

Work around

For now I'm making a quick remark plugin that replaces double quotes with &quot;! This definitely feels more like a bandaid than a real solution but want to share in case anyone hits this issue and needs a quick fix

import { Node } from 'unist';
import visit from 'unist-util-visit';

export function remarkImageAltQuotes() {
  return function transformer(tree: Node): void {
    visit(tree, ['image'], node => {
      const alt = node.alt as string;
      node.alt = alt.replace(/"/g, '&quot;');
    });
  };
}
@ChristianMurphy
Copy link
Member

Thanks for the detailed report @glitteringkatie! 🙇
And for providing an example, putting the example in a sandbox I'm seeing the same thing in MDX runtime https://codesandbox.io/s/clever-river-dko1k?file=/src/App.js

Though it does appear to be fixed already in XDM https://codesandbox.io/s/xdm-create-react-app-starter-forked-z8esj?file=/src/App.js

The fix in XDM appears to be https://github.com/wooorm/xdm/blob/fa8fb4918099ff085de45d03d01aa2f952228a8d/lib/plugin/recma-stringify.js#L103-L111
Which is not currently in place at

if (node.value != null) {
state.write('=')
this[node.value.type](node.value, state)
}

@ChristianMurphy ChristianMurphy added good first issue 👋 This may be a great place to get started! 🏡 area/internal This affects the hidden internals 💎 v2 Issues related to v2 🙆 yes/confirmed This is confirmed and ready to be worked on and removed 🔍 status/open labels Jun 17, 2021
@glitteringkatie
Copy link
Contributor Author

Ooh, awesome @ChristianMurphy! I was hoping it would be something like that. I am planning to get a fix in PR today or Monday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏡 area/internal This affects the hidden internals good first issue 👋 This may be a great place to get started! 🐛 type/bug This is a problem 💎 v2 Issues related to v2 🙆 yes/confirmed This is confirmed and ready to be worked on
Development

Successfully merging a pull request may close this issue.

2 participants