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

react-markdown replaces hrefs of links around it under some conditions #740

Closed
4 tasks done
arsinclair opened this issue May 1, 2023 · 7 comments
Closed
4 tasks done
Labels
👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on

Comments

@arsinclair
Copy link

arsinclair commented May 1, 2023

Initial checklist

Affected packages and versions

react-markdown@^8.0.7

Link to runnable example

https://github.com/arsinclair/react-markdown-error-repro

Steps to reproduce

So this bug report is something that has begun as a bizarre error, through two days of bisecting and troubleshooting I'm here to present no less strange circumstances when it occurs.

Description

Under some circumstances react-markdown transforms the value in the href attribute of the links on the same page, the links which should be 'invisible' to react-markdown.

For example, we have such component:

<>
    <p>
        <ReactMarkdown>
            {"[Jason Stanley](https://en.wikipedia.org/wiki/Jason_Stanley) is an American philosopher who is the Jacob Urowsky Professor of Philosophy at Yale University. He is best known for his contributions to philosophy of language and epistemology, which often draw upon and influence other fields, including linguistics and cognitive science."}
        </ReactMarkdown>
    </p>
    <p>
        <a href="https://upload.wikimedia.org/wikipedia/commons/f/f0/Jason_Stanley.jpg">
            Photo
        </a>
    </p>
</>

The href attribute on the rendered page should point to the image of Jason Stanley, however when we build the site and run it, the link will point to the the Wikipedia page about him. I.e. the target of the link inside of the React Markdown component gets copied into other links on the page.

These circumstances are:

  • Using Gatsby v3; it doesn't reproduce on Gatsby v4;
  • React Markdown needs to be inside of the <p> tag; the other link needs to be inside of another <p> tag;
  • The text in the React Markdown component needs to contain a link in markdown format;
  • This doesn't reproduce in the Dev Server mode of Gatsby, it only does in the production build;
  • The static version of the page generated (SSR) has the correct target of the link, the link attribute gets replaced only during hydration;

How it looks when the error is reproduced:

image

How it should be

image

Reproduction steps

  1. Download the linked reproduction repository
  2. Install npm dependencies
  3. Run gatsby build, and when it completes, run gatsby serve
  4. Open the Index page in the browser and check the href attribute of a second link

Resolution

So I am not sure where the error originates. I've looked into the code of react-markdown a little, but everything looks benign. Since these circumstances are very rare, the used version of Gatsby is quite old, and nested <p> tags is not even a valid HTML, I think the decision whether a time should be spent on fixing this is debatable. However, if this is indeed a problem with react-markdown, I think it's worth looking into, as it can be indicative of a scope spillover by the plugin, which can reveal other issues.

As a solution for my project I'm just going to get rid of an extraneous <p> tag.

Runtime

Node v16

Package manager

npm 8

OS

Linux

Build and bundle tools

Gatsby

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels May 1, 2023
@ChristianMurphy
Copy link
Member

ChristianMurphy commented May 1, 2023

Welcome @arsinclair!
Sorry you ran into a spot of trouble.

I'd recommend taking some time to try to isolate the issue.
I tried opening the code you shared in a sandbox https://codesandbox.io/s/github/arsinclair/react-markdown-error-repro and the link renders as expected.

I also copied your code into the create-react-app sandbox that is offered in the PR template.
https://stackblitz.com/edit/github-sttbsn?file=src%2Fapp.tsx

source code from sandbox
import React from 'react';
import Markdown from 'react-markdown';

const markdownSource = `
[Jason Stanley](https://en.wikipedia.org/wiki/Jason_Stanley) is an American philosopher who is the Jacob Urowsky Professor of Philosophy at Yale University. He is best known for his contributions to philosophy of language and epistemology, which often draw upon and influence other fields, including linguistics and cognitive science.
`;

const App = () => (
  <>
    <p>
      <Markdown>{markdownSource}</Markdown>
    </p>
    <p>
      <a href="https://upload.wikimedia.org/wikipedia/commons/f/f0/Jason_Stanley.jpg">
        Photo
      </a>
    </p>
  </>
);

export default App;

And I found that the issue doesn't seem to happen outside of Gatsby either.


Broadly speaking react-markdown does not edit the surrounding content.
It is most likely another tool in your environment causing the issue, I'd recommend taking some time to further isolate the issue.

I'm closing this as it doesn't appear to be an issue here.
Happy to re-open if you do find a way to replicate the issue, and it is somehow related to react-markdown.

If you have general questions, feel free to reach out in the Q&A discussions https://github.com/orgs/remarkjs/discussions/new?category=q-a

@ChristianMurphy ChristianMurphy closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2023
@github-actions

This comment has been minimized.

@ChristianMurphy ChristianMurphy added the 👀 no/external This makes more sense somewhere else label May 1, 2023
@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels May 1, 2023
@arsinclair
Copy link
Author

arsinclair commented May 1, 2023

Happy to re-open if you do find a way to replicate the issue, and it is somehow related to react-markdown.

The replication steps are provided in the original description. There's a repository that has the exact configuration where this issue is reproduced. I am not familiar with Code Sandbox, but the issue is reproduced on a local system.

I stripped it down to the bare minimum. There's nothing else I can do to further isolate it, without getting into the react-markdown internals.

It is most likely another tool in your environment causing the issue

It's very well reproduced on Netlify environment, so I can't say there's another tool in my system that's causing this. I can host it on Netlify, if it makes it easier for you.

@ChristianMurphy
Copy link
Member

The replication steps are provided in the original description.

Your original description also points to Gatsby being the issue.

  • Using Gatsby v3; it doesn't reproduce on Gatsby v4;
  • This doesn't reproduce in the Dev Server mode of Gatsby, it only does in the production build;
  • The static version of the page generated (SSR) has the correct target of the link, the link attribute gets replaced only during hydration;

Opening an issue at react-markdown will not fix an issue in gatsby.
Moreover, it sounds like an issue Gatsby already fixed in versions 4+.
Upgrade your version.

I stripped it down to the bare minimum. There's nothing else I can do to further isolate it, without getting into the react-markdown internals.

You believe it is an issue in react-markdown, but show a heap of evidence it's in gatsby.
If you want to show it's in react-markdown, show a way to replicate the issue without gatsby v3's large and notoriously quirky build system.

It's very well reproduced on Netlify environment

If you believe it is a Netlify issue, feel free to open an issue with them.

@arsinclair
Copy link
Author

arsinclair commented May 2, 2023

Yes, it's reproduced only with Gatsby and only with v3. And as I said, I am not sure where the error originates. It may very well be a Gatsby issue, or it may be an issue with react-markdown under very rare and unusual circumstances that Gatsby creates for it when it bundles the code.

Either way, the quick fix for me was to get rid of invalid HTML, i.e. nested <p> tags. I only raised this issue so that the maintainers might wish to look into it further, and for other people who may stumble on the same quirky behaviour.

It's totally fine if you deem that further investigation is not necessary at this point. Thank you for quick responses.

@ChristianMurphy
Copy link
Member

Yes, it's reproduced only with Gatsby and only with v3. And as I said, I am not sure where the error originates.

Then take some time to isolate it.
Folks here are happy to offer pointers, but open source does not exist to debug your code for you.

that Gatsby creates for it when it bundles the code.

In general unified, remark, and react-markdown do not, and will not, add work around for broken or misconfigured bundlers.
That should be fixed in the bundler or configuration itself.

Either way, the quick fix for me was to get rid of invalid HTML, i.e. nested

tags.

Invalid HTML breaks SSR platforms, this is well established #731 and #704
Gatsby/Next do not fix invalid HTML serverside, they send the invalid HTML to the browser, and the browser fixes it (#731 (comment)), which changes the structure the hydrator needs to update, and can cause unexpected behavior.

Gatsby should have thrown a hydration error here, because the HTML changed between initial render and hydration.

@arsinclair
Copy link
Author

Invalid HTML breaks SSR platforms, this is well established #731 and #704

This is good to know, thank you for the linked issues and for the explanation.

Then take some time to isolate it.

I already spent several hours debugging the hydration code in Gatsby to no avail. It's not an intuitive code and I can't find a good documentation about it. As clear as it is right now, the issue originates because of the invalid HTML, so that's enough an explanation for me: HTML is a tool, and if the tool is misused I am not surprised that something misbehaves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

2 participants