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

Invalid hook call when using react class component in MDX #9905

Closed
6 of 7 tasks
Danielku15 opened this issue Mar 2, 2024 · 12 comments
Closed
6 of 7 tasks

Invalid hook call when using react class component in MDX #9905

Danielku15 opened this issue Mar 2, 2024 · 12 comments
Labels
closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) external This issue is caused by an external dependency and not Docusaurus.

Comments

@Danielku15
Copy link

Danielku15 commented Mar 2, 2024

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

When you try to declare react components using React.Component in MDX files you end up with an Invalid hook call. error.

Reproducible demo

No response

Steps to reproduce

  1. Setup an empty Docusuarus with latest dependencies
    1. npx create-docusaurus@latest my-website classic --typescript && npx ncu -u && npm i && npm start
    2. Open https://docusaurus.new/stackblitz-ts
  2. Open the docs/tutorial-basics/markdown-features.mdx
  3. Change the file content to
---
sidebar_position: 4
---

import React from 'react'
export class Test extends React.Component {
	render() {
		return (<div>Title</div>);	
	}
}

<Test />
  1. Open the page in the browser.
  2. Experience the error.

Expected behavior

It should be possible declare React class components in MDX files without errors.

Actual behavior

The page crashes with:

Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
    at Object.throwInvalidHookError (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:16227:9)
    at Object.useContext (webpack-internal:///./node_modules/react/cjs/react.development.js:1619:21)
    at useMDXComponents (webpack-internal:///./node_modules/@mdx-js/react/lib/index.js:47:64)
    at Test.render (webpack-internal:///./docs/tutorial-basics/markdown-features.mdx:59:73)
    at finishClassComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:19747:31)
    at updateClassComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:19693:24)
    at beginWork (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:21606:16)
    at HTMLUnknownElement.callCallback (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:4164:14)
    at Object.invokeGuardedCallbackDev (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:4213:16)
    at invokeGuardedCallback (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:4277:31)

Moving the same class/code into a separate .tsx under src/ and importing it into the MDX works.

Your environment

  • Public source code: https://docusaurus.new/stackblitz-ts
  • Public site URL: not relevant
  • Docusaurus version used: 3.1.1
  • Environment name and version (e.g. Chrome 89, Node.js 16.4): Chrome 122, Node 20.9
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): Windows 11

Self-service

  • I'd be willing to fix this bug myself.
@Danielku15 Danielku15 added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Mar 2, 2024
@Josh-Cena
Copy link
Collaborator

Hi, I think this is an MDX issue (or maybe, design decision). The issue can be reproduced without Docusaurus:

// webpack.config.js
module.exports = {
  mode: "development",
  entry: "./index.tsx",
  output: {
    filename: "my-first-webpack.bundle.js",
  },
  module: {
    rules: [
      {
        test: /\.mdx$/,
        use: {
          loader: "@mdx-js/loader",
          options: {
            providerImportSource: "@mdx-js/react",
          },
        },
      },
      {
        test: /\.tsx?$/,
        use: "ts-loader",
        exclude: /node_modules/,
      },
    ],
  },
};
{/* doc.mdx */}
import React from 'react'
export class Test extends React.Component {
	render() {
		return (<div>Title</div>);	
	}
}

<Test />
// index.tsx
import React from "react";
import ReactDOM from "react-dom";
import Doc from "./doc.mdx";

ReactDOM.render(<Doc />, document.getElementById("root"));

The output produced contains the following:

render() {
    const _components = {
      div: "div",
      ...(0,_mdx_js_react__WEBPACK_IMPORTED_MODULE_2__.useMDXComponents)()
    };
    return (0,react_jsx_dev_runtime__WEBPACK_IMPORTED_MODULE_0__.jsxDEV)(_components.div, {
      children: \"Title\"
// ...

This is expected output per https://mdxjs.com/guides/injecting-components/. Please reach out to them to see if they want to support this.

@Josh-Cena Josh-Cena closed this as not planned Won't fix, can't repro, duplicate, stale Mar 3, 2024
@Josh-Cena Josh-Cena added closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) external This issue is caused by an external dependency and not Docusaurus. and removed bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Mar 3, 2024
@slorber
Copy link
Collaborator

slorber commented Mar 4, 2024

The output produced contains the following:

render() {
    const _components = {
      div: "div",
      ...(0,_mdx_js_react__WEBPACK_IMPORTED_MODULE_2__.useMDXComponents)()
    };
    return (0,react_jsx_dev_runtime__WEBPACK_IMPORTED_MODULE_0__.jsxDEV)(_components.div, {
      children: \"Title\"
// ...

Yes, using hooks in a class component is not going to work 😅

How do you get this output? This is surprising because the MDX playground doesn't give me this

CleanShot 2024-03-04 at 11 38 55

@slorber
Copy link
Collaborator

slorber commented Mar 4, 2024

Wondering what could be the compiled output of a React class component for MDX.

Particularly, if the user uses components provided by context:

import React from 'react'

export class Test extends React.Component {
	render() {
		return <SomeComponent/>
	}
}

Should the output be the following?

import React from 'react'

class Test extends React.Component {
  static contextType = MDXContext;

  render() {
    const {SomeComponent} = this.context;
    return <SomeComponent/>
  }
}

I don't see the MDXContext being exported @wooorm so I thought maybe class support has never been implemented?

https://mdxjs.com/packages/react/

@wooorm
Copy link
Contributor

wooorm commented Mar 4, 2024

I don’t think many people use class components with MDX 😅 Or at least maybe they do but don’t have issues with it? Also, not everyone uses context-based providers, and together, expect components to be injected from them in class methods.

I think the MDX issue here is explained here: mdx-js/mdx#2444 (comment), a solution is being thought up here: mdx-js/mdx#2445.

MDX also isn’t tied to React. So if the class story improves, we have to be careful to not mess with classes in other frameworks.

@slorber
Copy link
Collaborator

slorber commented Mar 4, 2024

I don’t think many people use class components with MDX 😅 Or at least maybe they do but don’t have issues with it? Also, not everyone uses context-based providers, and together, expect components to be injected from them in class methods.

Agree, that's a niche use case 😅

I think the MDX issue here is explained here: mdx-js/mdx#2444 (comment), a solution is being thought up here: mdx-js/mdx#2445.

I'm not sure @remcohaszing PR will fix this.

I don't see it in the Playground (maybe because it's framework agnostic?) but I see this in the dev tools, and there's no "conditional _createMdxContent" call being involved here.

import {Fragment as _Fragment, jsx as _jsx, jsxs as _jsxs} from "react/jsx-runtime";
import {useMDXComponents as _provideComponents} from "@mdx-js/react";
import React from 'react';
export class Test extends React.Component {
  render() {
    const _components = {
      div: "div",
      ..._provideComponents() // ❌ BAD
    };
    return _jsx(_components.div, {
      children: "Title"
    });
  }
}

We are simply calling a hook in a class component, which doesn't work.

Repro: https://stackblitz.com/edit/github-fq9usu?file=docs%2Fintro.md


MDX source:

---
sidebar_position: 1
---

# Tutorial Intro

import React from 'react'
export class Test extends React.Component {
	render() {
	return (<div>Title</div>);
	}
}

<Test />

Full module source code seen in DevTools:

import {Fragment as _Fragment, jsx as _jsx, jsxs as _jsxs} from "react/jsx-runtime";
import {useMDXComponents as _provideComponents} from "@mdx-js/react";
import React from 'react';
export class Test extends React.Component {
  render() {
    const _components = {
      div: "div",
      ..._provideComponents()
    };
    return _jsx(_components.div, {
      children: "Title"
    });
  }
}
export const toc = [];
function _createMdxContent(props) {
  const _components = {
    h1: "h1",
    ..._provideComponents(),
    ...props.components
  };
  return _jsxs(_Fragment, {
    children: [_jsx(_components.h1, {
      id: "tutorial-intro",
      children: "Tutorial Intro"
    }), "\n", "\n", "\n", _jsx(Test, {})]
  });
}
export default function MDXContent(props = {}) {
  const {wrapper: MDXLayout} = {
    ..._provideComponents(),
    ...props.components
  };
  return MDXLayout ? _jsx(MDXLayout, {
    ...props,
    children: _jsx(_createMdxContent, {
      ...props
    })
  }) : _createMdxContent(props);
}

@Danielku15
Copy link
Author

I don’t think many people use class components with MDX

https://github.com/search?q=%22export+class%22+React.Component+language%3Amdx&type=code

There are some false-positives (code snippets instead of components declared in MDX) in the result but it gives a high level insight data it is used here and there. But there are really not many results

I personally find it interesting that things work if you move the react class components into a dedicated file and import it. One would assume that the callstack and execution flows stay the same on rendering. Maybe this can give some hints on supporting class components within MDX? I assume MDX is injecting some additional code into the classes which cause the problem.

If class components cannot be supported (anymore) it might be worth to emit some user/dev friendly warnings/errors (configurable?). Even though I raised the issue I'm also fine with not having support for class components within MDX. I refactored my code to pull out the more complex components and things are working fine for me now. 😁

@remcohaszing
Copy link

remcohaszing commented Mar 4, 2024

This is different from mdx-js/mdx#2444 (comment).

I didn’t even realize MDX supports useMDXComponents outside of MDXContent / _createMdxContent. I could see both arguments why it should or shouldn’t. Personally I lean towards it shouldn’t, but changing it would be a breaking change.

Docusaurus could replace @mdx-js/react with a custom import provider, or even with a fallback to @mdx-js/react if the user really wants to, like Next.js. This avoids the use of hooks completely.

@wooorm
Copy link
Contributor

wooorm commented Mar 4, 2024

I don't see it in the Playground (maybe because it's framework agnostic?

The playground doesn’t use providers (@mdx-js/react). With that turned on, this behavior happens. Without, it doesn’t.

import { compile } from "@mdx-js/mdx";

const document = `
import React from 'react'

export class Test extends React.Component {
  render() {
    return (<div>Title</div>);
  }
}

<Test />`;

const result = await compile(document, {
  providerImportSource: "@mdx-js/react",
});

console.log(String(result));

I assume MDX is injecting some additional code into the classes which cause the problem.

Yes it does!

There are some false-positives (code snippets instead of components declared in MDX) in the result but it gives a high level insight data it is used here and there. But there are really not many results

I checked the first 15, and 13 were code blocks. The only 2 components are https://github.com/trytouca/trytouca/blob/0b16f242f5216fe3b49bb3248558e4b1a358308c/docs/docs/sdk/differences.mdx#L8 and https://github.com/ehsanghaffar/touca-core/blob/5ae23339a26813f1c3c8a95415b5c747529e664d/docs/docs/sdk/Readme.mdx#L6. So it seems like very little class components.

I didn’t even realize MDX supports useMDXComponents outside of MDXContent / _createMdxContent. I could see both arguments why it should or shouldn’t. Personally I lean towards it shouldn’t, but changing it would be a breaking change.

The behavior is intentional. People want to inject components into MDX files. That also applies to components in components and components defined in MDX.

However, the component injection is for a) undefined components (so <This>), and for b) overwriting markdown things (so # This), not for overwriting <div>. In this case, JSX in an export / expression, the b) example will never happen. But the code is injected anyway.

That is to say, regardless of class components or function components, the provider should not be injected for <div>.
Only if someone puts <This> in a function (whether function or class component), that should happen.
And I don’t consider fixing it as a breaking change as it’s the always intended behavior.


Why is the React context API not allowed in classes?

@slorber
Copy link
Collaborator

slorber commented Mar 4, 2024

The playground doesn’t use providers (@mdx-js/react). With that turned on, this behavior happens. Without, it doesn’t.

👍

That could be a useful new playground option to add?

That is to say, regardless of class components or function components, the provider should not be injected for <div>. Only if someone puts <This> in a function (whether function or class component), that should happen. And I don’t consider fixing it as a breaking change as it’s the always intended behavior.

That doesn't seem to be a breaking change for me either 🤔 This looks like a regular bug fix because anyway, atm a simple class component doesn't even work 😅 I don't see how you can break it more.

Why is the React context API not allowed in classes?

What do you mean?

Hooks are not allowed in class components, so useMDXComponents/_provideComponents() won' work.

But context remains supported with static contextType and this.context.

export class Test extends React.Component {
  render() {
    return <div><This/></div>
  }
}

should probably be compiled as:

import React from "react";
import {MDXContext} from "@mdx-js/mdx";

export class Test extends React.Component {
  static contextType = MDXContext;

  render() {
    const _components = {
      div: "div",
      ...this.context,
    };
    return _jsx(_components.div, {
      children: _jsx(_components.This),
    });
  }
}

But MDXContext is not exported atm.

@remcohaszing
Copy link

The problem with contextType is that you can only have one context type per class component. (Unless there’s something I don’t know?)

@slorber
Copy link
Collaborator

slorber commented Mar 4, 2024

Yes, otherwise you can use <MDXContext.Consumer> 😅

@wooorm
Copy link
Contributor

wooorm commented Mar 4, 2024

Hooks are not allowed in class components, so useMDXComponents/_provideComponents() won' work.

If you choose to use something other than @mdx-js/react, there could be anything happening in there. It’s more that @mdx-js/react has a hook (React.useContext(MDXContext)). Could we read from context, that would work in a class, some other way?

Consumer was indeed originally used: mdx-js/mdx@07e3a72#diff-b927989ca8df29e648b84decc4a008d4280568465bfec6e72548f1389d111669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) external This issue is caused by an external dependency and not Docusaurus.
Projects
None yet
Development

No branches or pull requests

5 participants