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

TypeError when using react-syntax-highlighter #666

Closed
4 tasks done
Katsukiniwa opened this issue Dec 26, 2021 · 15 comments · Fixed by #668
Closed
4 tasks done

TypeError when using react-syntax-highlighter #666

Katsukiniwa opened this issue Dec 26, 2021 · 15 comments · Fixed by #668
Labels
☂️ area/types This affects typings 🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on

Comments

@Katsukiniwa
Copy link

Katsukiniwa commented Dec 26, 2021

Initial checklist

Affected packages and versions

7.1.1

Link to runnable example

No response

Steps to reproduce

type error happened.

015

import ReactMarkdown from 'react-markdown';
import { Prism as SyntaxHighlighter } from 'react-syntax-highlighter';
import { darcula } from 'react-syntax-highlighter/dist/cjs/styles/prism';

const sample = () => {
  return (
    <ReactMarkdown
      components={{
        code({ node, inline, className, children, ...props }) {
          const match = /language-(\w+)/.exec(className || '');
          // I avoid with type casting like this.
          // const ref = props.ref as LegacyRef<SyntaxHighlighter> | undefined;

          return !inline && match ? (
            <SyntaxHighlighter style={darcula} language={match[1]} {...props}>
              {String(children).replace(/\n$/, '')}
            </SyntaxHighlighter>
          ) : (
            <code className={className}>{children}</code>
          );
        },
      }}
    >
      {post.content}
    </ReactMarkdown>
  )
}

Expected behavior

props.ref type should be LegacyRef<SyntaxHighlighter> | undefined

Actual behavior

no error found.

Runtime

Node v16

Package manager

No response

OS

macOS

Build and bundle tools

Next.js

@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 Dec 26, 2021
@ChristianMurphy
Copy link
Member

ChristianMurphy commented Dec 26, 2021

@Katsukiniwa thanks for reaching out, sorry you ran into a spot of trouble.
This isn't a bug in react-markdown it has to do with how @types/react-syntax-highlighter handles additional props being passed through {...props}, specifically ref.

see https://codesandbox.io/s/react-markdown-syntax-highlight-typescript-p9tw0 for an example, try adding and removing ref on line 25 to see the difference it makes.

code({ node, inline, className, children, ref, ...props }) {
                                          ^^^^
full example
import React from "react";
import Markdown from "react-markdown";
import { Prism as SyntaxHighlighter } from "react-syntax-highlighter";
import { darcula } from "react-syntax-highlighter/dist/cjs/styles/prism";

const markdownSource = `
# heading

* list
* items

\`\`\`js
function () {}
\`\`\`
`;

const App = () => (
  <Markdown
    components={{
      code({ node, inline, className, children, ref, ...props }) {
        const match = /language-(\w+)/.exec(className || "");

        return !inline && match ? (
          <SyntaxHighlighter style={darcula} language={match[1]} {...props}>
            {String(children).replace(/\n$/, "")}
          </SyntaxHighlighter>
        ) : (
          <code className={className}>{children}</code>
        );
      }
    }}
  >
    {markdownSource}
  </Markdown>
);

export default App;
```

</details>

@ChristianMurphy ChristianMurphy added the 🙋 no/question This does not need any changes label Dec 26, 2021
@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 Dec 26, 2021
@ChristianMurphy ChristianMurphy added the ☂️ area/types This affects typings label Dec 26, 2021
@Katsukiniwa
Copy link
Author

Katsukiniwa commented Dec 27, 2021

@ChristianMurphy
Thank you, it worked!!
The sample code on the README.md, ref is not passed as argument. Is this a wrong something?
If it is something bug, may I fix it and create pull reqeuest?

https://github.com/remarkjs/react-markdown#use-custom-components-syntax-highlight

@ChristianMurphy
Copy link
Member

The example in the readme is JavaScript.
Passing ref doesn't cause a runtime error, just a type(script) error.

Thoughts @wooorm or @remcohaszing on changing the example to TS? 💭

@remcohaszing
Copy link
Member

I think this could be a bug in react-markdown. This depends on how code() is called. I’m not very familiar with this particular package.

On the following lines JSX.IntrinsicElements is used to get prop types.

* @typedef {JSX.IntrinsicElements['code'] & ReactMarkdownProps & {inline?: boolean}} CodeProps
* @typedef {JSX.IntrinsicElements['h1'] & ReactMarkdownProps & {level: number}} HeadingProps
* @typedef {JSX.IntrinsicElements['li'] & ReactMarkdownProps & {checked: boolean|null, index: number, ordered: boolean}} LiProps
* @typedef {JSX.IntrinsicElements['ol'] & ReactMarkdownProps & {depth: number, ordered: true}} OrderedListProps
* @typedef {JSX.IntrinsicElements['table'] & ReactMarkdownProps & {style?: Record<string, unknown>, isHeader: boolean}} TableCellProps
* @typedef {JSX.IntrinsicElements['tr'] & ReactMarkdownProps & {isHeader: boolean}} TableRowProps
* @typedef {JSX.IntrinsicElements['ul'] & ReactMarkdownProps & {depth: number, ordered: false}} UnorderedListProps

This is correct if code is called as a function, but incorrect if it’s called as React JSX. In that case the types should be:

/**
 * @typedef {React.ComponentPropsWithoutRef<'code'> & ReactMarkdownProps & {inline?: boolean}} CodeProps
 */

@ChristianMurphy
Copy link
Member

It is rendered here:

// Ensure no React warnings are emitted for void elements w/ children.
return children.length > 0
? React.createElement(component, properties, children)
: React.createElement(component, properties)

and is reflecting the intrinsic type here:
export type NormalComponents = {
[TagName in keyof JSX.IntrinsicElements]:
| keyof JSX.IntrinsicElements
| ComponentType<JSX.IntrinsicElements[TagName] & ReactMarkdownProps>
}

and I correct in interpreting your comment to mean

// in react-markdown/lib/complex-types.ts line 27
JSX.IntrinsicElements[TagName]
// could be replaced with
React.ComponentPropsWithoutRef<TagName>

?

@remcohaszing
Copy link
Member

That seems correct more than the types I referenced, but indeed the problem is what I suspected: JSX.IntrinsicElements is used where React.ComponentPropsWithoutRef should be used.

ChristianMurphy added a commit to ChristianMurphy/react-markdown that referenced this issue Dec 31, 2021
@ChristianMurphy ChristianMurphy mentioned this issue Dec 31, 2021
5 tasks
ChristianMurphy added a commit to ChristianMurphy/react-markdown that referenced this issue Dec 31, 2021
@ChristianMurphy ChristianMurphy linked a pull request Dec 31, 2021 that will close this issue
5 tasks
@ChristianMurphy
Copy link
Member

@Katsukiniwa you could also give version 7.1.2 a try, it should not require any tinkering with ref in typescript.

@Katsukiniwa
Copy link
Author

@ChristianMurphy
Thank you !!!

@18601673727
Copy link

If I'm reading this issue correctly, at this momenet, on version 9.0.0, the same problem is still there and README.md is still misleading people

@ChristianMurphy
Copy link
Member

ChristianMurphy commented May 3, 2024

Hey again @18601673727! 👋
You aren't reading the issue correctly.
The example in the readme is js, not ts.

And the compatability issue in the types was from several react versions ago, you are seeing something something relayed but different.

Likely you are using an incompatible version of the react types. Version 18 is currently supported.

"@types/react": "^18.0.0",

@18601673727
Copy link

Hello @ChristianMurphy, sorry being so blunt about it, but my current package.json is here:

  "dependencies": {
    "@ethereumjs/util": "^9.0.3",
    "@metamask/providers": "^16.0.0",
    "@nextui-org/react": "^2.3.6",
    "@nextui-org/system": "2.1.2",
    "@nextui-org/theme": "2.2.3",
    "@prisma/client": "5.13.0",
    "@types/node": "20.5.7",
    "@types/react": "18.3.1",
    "@types/react-dom": "18.3.0",
    "autoprefixer": "10.4.19",
    "clsx": "^2.0.0",
    "date-fns": "^3.6.0",
    "eslint": "8.48.0",
    "eslint-config-next": "14.2.1",
    "framer-motion": "^11.1.7",
    "i18next": "^23.11.2",
    "intl-messageformat": "^10.5.0",
    "md-editor-rt": "^4.13.5",
    "next": "14.2.3",
    "next-auth": "5.0.0-beta.16",
    "next-remove-imports": "^1.0.12",
    "next-themes": "^0.3.0",
    "postcss": "8.4.38",
    "posthog-js": "^1.129.0",
    "react": "18.2.0",
    "react-dom": "18.2.0",
    "react-hot-toast": "^2.4.1",
    "react-markdown": "^9.0.1",
    "react-nice-avatar": "^1.5.0",
    "react-syntax-highlighter": "^15.5.0",
    "remark-breaks": "^4.0.0",
    "remark-gfm": "^4.0.0",
    "safe-marked": "^16.0.0",
    "sharp": "^0.33.3",
    "tailwind-merge": "^2.3.0",
    "tailwind-variants": "^0.2.1",
    "tailwindcss": "3.4.3",
    "typescript": "5.4.5",
    "zod": "^3.22.5",
    "zod-i18n-map": "^2.27.0",
    "zod-validation-error": "^3.2.0"
  },
  "devDependencies": {
    "@tailwindcss/typography": "^0.5.13",
    "@types/react-syntax-highlighter": "^15.5.12",
    "prisma": "^5.13.0",
    "ts-node": "^10.9.2"
  }

I know all this open sourced "compiler work" weights a lot, and maintainers like you deserve huge amount of respect, but we all know that it's 2024 already and typescript is more or less a default kind of thing to README.md about, upstream issue happens but considering the user base react-markdown has of now, we might be obliged to handle theirs shortcomings. So it's either modify README.md on this side or don't recommand that react-syntax-highlight no more. Or we would expect this kind of problem happen again.

@ChristianMurphy
Copy link
Member

ChristianMurphy commented May 3, 2024

we all know that it's 2024 already and typescript is more or less a default kind of thing to README.md about

While I like and use TypeScript, I respectfully disagree.
JavaScript is still the go to for documentation.
It is in:

So it's either modify README.md on this side or don't recommend that react-syntax-highlight no more. Or we would expect this kind of problem happen again.

The third, and best option, is to look into the root of the problem, and open a fix to @types/react or react-syntax-highlight.
This is a types regression that happened elsewhere, this has worked fine for three years.

I know all this open sourced "compiler work" weights a lot, and maintainers like you deserve huge amount of respect, but [...]

No "but".
This isn't the place or the time to make demands or state your opinion as a fact.
We're working to help you and the community as a whole, rushing incomplete half fixes with incomplete information doesn't help.

Take the time to frame your problem https://github.com/remarkjs/.github/blob/main/support.md, share a runnable example of the issue you are seeing (https://stackblitz.com/github/remarkjs/.github/tree/main/sandbox-templates/react-markdown-with-vite is good starting point), and consider the alternatives.

@18601673727
Copy link

18601673727 commented May 3, 2024

Are you saying "@types/react": "18.3.1", is not compatible enough for "@types/react": "^18.0.0" as per you suggested? Why my VSCode is still complaining with that piece of code your README.md documented? All this is a simple coincidence or ignorance/arrogance? As a member of 12k-star opensource project, bothering his downstream users by not willing to make a teeny tiny modification in the documentation then lecturing people how and when to communicate? You know what, don't do any changes, leave it as it is. 👎

@ChristianMurphy
Copy link
Member

ChristianMurphy commented May 3, 2024

Are you saying "@types/react": "18.3.1", is not compatible enough for "@types/react": "^18.0.0" as per you suggested?

Nowhere did I say that, making things up just to argue will get you blocked from the community.

Why my VSCode is still complaining with that piece of code your README.md documented?

Share a reproducible example of what you are seeing as I asked you to above, and we can find out together.

All this is a simple coincidence or ignorance/arrogance?

Likely, right now you seem to be both.
Work with the people trying to help you instead of fighting them.

As a member of 12k-star opensource project, bothering his downstream users by not willing to make a teeny tiny modification in the documentation then lecturing people how and when to communicate?

I'm not willing to risk breaking code or misleading 12k+ people because one angry commenter, who refuses give an example of what they are seeing, disagrees with part of the documentation.
Show the problem, discuss what you have tried, and we can discuss solutions.

@wooorm
Copy link
Member

wooorm commented May 3, 2024

@18601673727 you are embarrassing yourself. This is issues. You are commenting on other people's issues and picking fights. Last warning. Be constructive and follow orders or be blocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☂️ area/types This affects typings 🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging a pull request may close this issue.

5 participants