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

MDX Loader treats .md files like markdown even when extension is configured to load them as MDX #2302

Closed
4 tasks done
joostdecock opened this issue May 14, 2023 · 6 comments
Closed
4 tasks done
Labels
🙋 no/question This does not need any changes

Comments

@joostdecock
Copy link
Contributor

Initial checklist

Affected packages and versions

@mdx-js/loader 2.3.0 (most recent)

Link to runnable example

https://github.com/joostdecock/mdxmd

Steps to reproduce

The repo linked above is a minimal reproduction. Steps to run it:

git clone git@github.com:joostdecock/mdxmd.git
cd mdxmd
npm i
npm run dev

Now open your browser at http://localhost:3000 and you will see this:

result

Expected behavior

The next config is as follows:

const configuration = {
  pageExtensions: ["mjs"],
  webpack(config, options) {
    config.module.rules.push({
      test: /\.mdx?$/,
      use: [
        options.defaultLoaders.babel,
        {
          loader: "@mdx-js/loader",
          /** @type {import('@mdx-js/loader').Options} */
          options: {
            /* jsxImportSource: …, otherOptions… */
          },
        },
      ],
    });

    return config;
  },
};

Note the the test which configures both .md and .mdx extensions to be treated the same:

test: /\.mdx?$/,

When files are loaded like this:

import MD from '../mdx/example.md'
import MDX from '../mdx/example.mdx'

I would expect both of them to be treated as MDX.

Actual behavior

When files are loaded like this:

import MD from '../mdx/example.md'
import MDX from '../mdx/example.mdx'

the .md file is treated as markdown whereas the .mdx file is treated as MDX.

Runtime

Node v16

Package manager

npm v8

OS

Linux

Build and bundle tools

webpack

@joostdecock
Copy link
Contributor Author

The issue template tends to make it easy to fall into the XY-problem trap, so to be clear, let me state what I would like to do:

I have a bunch of .md files with MDX content. I want to load those in NextJS as MDX. But they get loaded as Markdown.
If I rename the file to .mdx it works as expected, with the exact same config.

Renaming all files is something I'd rather avoid. Instead, I'd like to say: *Hey, treat these .md files as .mdx files, but configuring extension accordingly does not seem to do the trick.

@ChristianMurphy
Copy link
Member

ChristianMurphy commented May 14, 2023

@joostdecock this is the expected behavior.
MDX can process plain markdown or MDX.
How it processes is configured by the format option. https://mdxjs.com/packages/mdx/#optionsformat
You have configured your parser (implicitly) to detect which will look at the file type and choose which format to use based off the extension.
If you would like everything to be treated as MDX, set the format option to mdx.


though taking a few steps back, it could be an even better idea to use the correct extension in the first place 🙂

@ChristianMurphy ChristianMurphy added the 🙋 no/question This does not need any changes label May 14, 2023
@joostdecock
Copy link
Contributor Author

@ChristianMurphy Oh wow thank you so much because I was starting to go insane here :)

I don't know if I'm the only one dumb enough not to figure this out, but perhaps a note in the docs would help?

To be fair, I was initially using @next/mdx which handles the webpack config. The documentation (linked to from the MDX loader docs) states:

Optionally you can match other file extensions for MDX compilation, by default only .mdx is supported

// next.config.js
const withMDX = require('@next/mdx')({
extension: /.(md|mdx)$/,
})
module.exports = withMDX()

Now I am not a native speaker, but to me reading match other file extensions for MDX compilation sounds like this is all I need to compile .md files as MDX.

Not complaining. I'm very appreciative of all the great work and prompt feedback. I'm merely pointing out that it's not that I didn't bother to read the docs. It's that what was written in them made me feel it should work 🤷

@ChristianMurphy
Copy link
Member

Optionally you can match other file extensions for MDX compilation, by default only .mdx is supported

to me reading match other file extensions for MDX compilation sounds like this is all I need to compile .md files as MDX.

I'd agree that the Next.js documentation worded in a way that can be misinterpreted, would you be interested in opening a PR to Next.js to clarify their documentation?

@joostdecock
Copy link
Contributor Author

@ChristianMurphy Already on it!

joostdecock added a commit to joostdecock/next.js that referenced this issue May 14, 2023
…iles as MDX

I got bitten by this passage because it reads as if merely changing the `extension` setting will cause `.md` files to be loaded as MDX.

See: mdx-js/mdx#2302
@joostdecock
Copy link
Contributor Author

vercel/next.js#49785

ijjk added a commit to vercel/next.js that referenced this issue Jun 14, 2023
…iles as MDX

I got bitten by this passage because it reads as if merely changing the
`extension` setting will cause `.md` files to be loaded as MDX.

See: mdx-js/mdx#2302

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

---------

Co-authored-by: JJ Kasper <jj@jjsweb.site>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 no/question This does not need any changes
Development

No branches or pull requests

2 participants