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

Fix Next by using default import for CJS #73

Merged
merged 2 commits into from
Jul 30, 2021
Merged

Conversation

danielolaviobr
Copy link
Contributor

Using named imports was rendering an error when using with Next.js and mdx-bundler, the following change fixes the problem

Using named imports was rendering an error when using with Next.js and mdx-bundler, the following change fixes the problem
Copy link
Collaborator

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense for the currently version range https://unpkg.com/mdast-util-mdx@0.1.0/index.js

An alternative could be updating

"mdast-util-mdx": "^0.1.0",
to ^1.0.0
which is pure ESM and has no default export https://github.com/syntax-tree/mdast-util-mdx/blob/b27dc8aa3ff340815b5f04f15ee1e6ed3a753a79/index.js#L13-L23

Copy link
Owner

@wooorm wooorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This is an error in Next + webpack et al, which you could perhaps report there. While CJS handling is non-standard, I believe users are right to expect Next/webpack to work the same as Node for CJS in ESM.
  • I’m not sure whether the next version of mdast-util-mdx already works, it might expect a couple of updates in micromark/mdast/remark/unified, so we should probably wait a little bit before remark is updated
  • pkg is a bit of a vague name, and the code style doesn’t match this project: here’s what I suggest, but you can also check these things locally by running npm test

lib/plugin/remark-mdx.js Outdated Show resolved Hide resolved
Co-authored-by: Titus <tituswormer@gmail.com>
@wooorm wooorm changed the title Change named imports to default import Fix Next by using default import for CJS Jul 30, 2021
@wooorm wooorm merged commit 5ce5208 into wooorm:main Jul 30, 2021
@wooorm
Copy link
Owner

wooorm commented Jul 30, 2021

danielolaviobr added a commit to danielolaviobr/mdx-bundler that referenced this pull request Jul 30, 2021
Update xdm to patch the default import error, as explained in the following (PR)[wooorm/xdm#73]
kentcdodds pushed a commit to kentcdodds/mdx-bundler that referenced this pull request Jul 30, 2021
Update xdm to patch the default import error, as explained in the following (PR)[wooorm/xdm#73]
Kusou1y1 added a commit to Kusou1y1/kentcdoddsl that referenced this pull request Dec 8, 2021
Update xdm to patch the default import error, as explained in the following (PR)[wooorm/xdm#73]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants