-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
Content plugins should prevent usage of a parent folder as mdx source path #9027
Comments
I don't think we should restrict users as you suggested. I believe it should be possible in certain cases (like mine currently). It can be useful to reuse the original documentation that was used in a project. For example, a company might have built their documentation starting from the README and other folders in their Git repo. If they want to add Docusaurus and use the README as the Docusaurus intro, they wouldn't be able to do so. I'm pretty sure this is a bad idea, because sometimes Docusaurus can be used simply to "beautify" the documentation that was originally done in the Git tree. It's important to allow this flexibility so as not to hinder users who want to leverage Docusaurus to enhance their existing documentation. |
Something worth mentioning is that Docusaurus uses MDX, and MDX compiles Markdown documents to React components. For this reason, the MDX React components must be able to find the require We do have a fallback to support rendering But you shouldn't try to render Do you want the freedom to render any md file from anywhere in your filesystem? You will have to use Webpack raw-loader (importing md content as pure strings instead of React components) and then use a runtime Markdown library to import/render that raw string inside other MDX docs. Or create your own plugin that's not based on MDX. # This is an MDX doc in `website/docs`
import RenderMarkdown from 'runtime-markdown-lib';
import ImportedMarkdownDocString from '!!raw-loader!/anywhere/in/my/filesystem.md';
<RenderMarkdown>{MyComponentSource}</RenderMarkdown> Hope this makes sense. |
TL;DRThe issue is not as generic. It's only about Yes, MDX files are compiled into a js code that imports react, and for things to work, those imports to react must resolve to the same location that react resolves inside the app. But isn't that the limit? Consider a case like this: .
├── website/ # (docusaurus)
│ └── node_modules/
├── docs/ # (docs folder)
│ └── doc.mdx
└── node_modules/ As you mentioned too, it works if dependencies are hoisted. But more specifically, it works if react is hoisted, which makes react resolve to the same location in these two cases:
Which is basically a requirement because of which react and react-dom are peer dependencies. But the difference in case of mdx files is that the import to react is in a generated code (the react component the mdx content compiles to), as opposed to imports to react from components in npm packages, where the peer dependency requirement is ensured by the package manager. A solution is to add an alias for
Here is a full example of how disabling hoisting breaks things and how the aliasing solution fixes it: https://codesandbox.io/p/devbox/charming-river-x776sj |
@alirezamirian that looks like a reasonable suggestion. I've implemented it here with a secret env variable to opt-out just in case it cause troubles to our users for unexpected reasons: #10391 Note: I'm keeping this issue open because we should still forbid users to have one docs folder nested into another. I also doubt using the site root as docs plugin source folder is a good idea. The aliasing suggestion is mostly useful to solve problems where docs are outside the website folder / monorepo. |
Note we also need to alias import {useMDXComponents as _provideComponents} from '@mdx-js/react' Note you can also implement the solution yourself in userland: function plugin() {
return {
name: "my-plugin",
configureWebpack() {
return {
resolve: {
alias: {
// The trick to use "/package.json" is because require.resolve would resolve to the exact js file, but we want the path to the directory where react is installed.
react: path.dirname(require.resolve("react/package.json")),
"react-dom": path.dirname(
require.resolve("react-dom/package.json"),
),
"@mdx-js/react": path.dirname(require.resolve("@mdx-js/react")),
},
},
};
},
};
} |
Awesome! Thanks!
Yes, exactly, the aliasing is relevant for docs folder outside website, which is a superset of docs folder being a parent folder. I actually have some observations and suggestions regarding the particular case of a parent folder being set as docs root. I'm planning to write here later. I hope I make it in time before you officially forbid that :D |
True. In fact I have done that already in my actual project, but had forgotten to update my comment :) |
Have you read the Contributing Guidelines on issues?
Motivation
Some users attempt to do:
See #9002
This is bad because using the site root (or any of its parent) as the source of md/mdx content will register a mdx loader for that folder, and lead to conflicts with the blog/page plugins that are nested inside that folder.
The error message is not very clear, such as:
I think we should improve the DX and fail-fast, forbidding the use of the site folder (or any of its parent) as content (md/mdx) source root.
Eventually the error message could mention that if user wants to have some content folders upper in the tree (like it was in Docusaurus v1:
../docs
), it's possible to use that specific folder inside a parent dir. It can be useful to also mention multi-instance as an option (see #9002 (reply in thread))Technically it might be possible to order mdx-loaders in an order that could make it work, but I doubt it's a good idea to allow that anyway and could lead to more annoying problems too, for example users trying to use autogenerated docs but it picks content from blog/pages too... So I think it's safer to just forbid this entirely.
To generalize: the problem is to have one content folder nested inside another. Unfortunately due to the modular nature of Docusaurus (the concept of a "content folder" is plugin-specific), it's probably not possible to generalize the solution and we can only prevent he most common case: declaring a direct site parent folder as content folder.
Self-service
The text was updated successfully, but these errors were encountered: