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

Objects are ignored in remarkPlugins #2136

Closed
4 tasks done
gaearon opened this issue Sep 24, 2022 · 1 comment
Closed
4 tasks done

Objects are ignored in remarkPlugins #2136

gaearon opened this issue Sep 24, 2022 · 1 comment
Labels
👀 no/external This makes more sense somewhere else

Comments

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2022

Initial checklist

Affected packages and versions

@mdx-js/mdx@2.1.3

Link to runnable example

No response

Steps to reproduce

This neither crashes nor works.

const code = await compile(source, {
  remarkPlugins: [
    await import('some-remark-plugin')
  ]
});

Expected behavior

The issue is that import returns an object like { default: ... }. I forgot about that. I didn't check it because I expected that MDX would crash when given an invalid plugin. My request is to crash if the plugin isn't a function instead of ignoring it.

Actual behavior

MDX happily skipped my invalid plugin without attempting to run it. So I didn't know there is a problem. (I spent 30 minutes on this.)

Runtime

Node v17

Package manager

npm v8

OS

macOS

Build and bundle tools

Next.js

@wooorm
Copy link
Member

wooorm commented Sep 24, 2022

My request is to crash if the plugin isn't a function instead of ignoring it.

Thing is: we supports presets (which are objects): collections of plugins and/or settings. In this case, it’s seen as a preset without settings, without plugins, and with an unknown field.

https://github.com/unifiedjs/unified/blob/144eec01b0e1faa23655359d61c3749ad2af99e7/index.d.ts#L551-L558

So we can’t throw on objects.

I spent 30 minutes on this.

Yeah, makes sense, I’m assuming other folks will also run into this.

We could throw on objects that have neither plugins nor settings fields though.
I opened unifiedjs/unified#200. There are some downsides but I think it’s good to have. I think it should go in a major, and I don’t think it’s worth it to cut a major just for this. So I expect it to land, but not soon!

@wooorm wooorm closed this as completed Sep 24, 2022
@wooorm wooorm added the 👀 no/external This makes more sense somewhere else label Sep 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 no/external This makes more sense somewhere else
Development

No branches or pull requests

2 participants