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

Add new transformer for MDX v2 #8625

Open
wants to merge 6 commits into
base: v2
Choose a base branch
from
Open

Add new transformer for MDX v2 #8625

wants to merge 6 commits into from

Conversation

Ayc0
Copy link

@Ayc0 Ayc0 commented Nov 12, 2022

↪️ Pull Request

Other variant for #7922 to solve #7934.
The main differences are:

  • we can have some custom config for MDX,
  • pushed a new package @parcel/transformer-mdx-2 instead of modifying the existing.

For the MDX config, you can add a .mdxrc.js or .mdxrc.cjs file at the root of your repo to be able to customise MDX's behavior (see https://mdxjs.com/packages/mdx/#optionsremarkplugins).

Note: I tried to build parcel locally but yarn build-native gets stuck. So i couldn't test this PR locally

Note 2: to make it work, we also need @parcel/transformer-js as MDX v2 generates ESM code:

{
  "extends": "@parcel/config-default",
  "transformers": {
    "*.mdx": [ "parcel-transformer-mdx-2", "@parcel/transformer-js" ]
  }
}

💻 Examples

Previous MDX examples should still be applicable.

I also added a example here with some MDX config too: https://github.com/Ayc0/test-parcel/blob/7d05ca9/src/hello.mdx

🚨 Test instructions

Previous MDX tests should still be applicable.
But I also added a new one (not really sure if this is used).

I also created a repo here: https://github.com/Ayc0/test-parcel/blob/7d05ca9/src/hello.mdx

Notice the MDX 2 specific syntaxes like {a} or the MD in HTML, and that the strikethrough and the table work (provided by GFM):

Input Config Output
image image image

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@Ayc0 Ayc0 marked this pull request as ready for review November 12, 2022 18:34
@Ayc0 Ayc0 marked this pull request as draft November 12, 2022 20:53
@Ayc0
Copy link
Author

Ayc0 commented Nov 12, 2022

On rebuild, parcel crashes with this error:

image

This might be a bug within Parcel: the import resolves the first time:

image

But after a recompilation, it crashes.

Edit: it only happens in parcel 2.8.0, in parcel 2.7.0 it works fine:

Screen.Recording.2022-11-13.at.01.32.57.mov

@Ayc0 Ayc0 marked this pull request as ready for review November 13, 2022 01:11
@devongovett
Copy link
Member

That is caused by v8-compile-cache I believe...

@mischnic
Copy link
Member

module.exports = new Transformer({
async loadConfig({config}) {
// config.getConfig only works with CJS
let configFile = await config.getConfig(['.mdxrc.js', '.mdxrc.cjs'], {
Copy link
Author

Choose a reason for hiding this comment

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

To follow https://github.com/mdx-js/mdx/blob/v1/packages/parcel-plugin-mdx/src/MDXAsset.js#L13, I could add mdx.config.js and mdx.config.cjs.

Or just use those 2 instead of .mdxrc.js (as usually the rc files are JSON files, not JS files)

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