Mdx: Forwarding options to remarkPlugins/rehypePlugins can break them

Created on 12 May 2019  路  6Comments  路  Source: mdx-js/mdx

Subject of the issue

Currently @mdx-js/mdx forwards its options to every plugin in remarkPlugins and rehypePlugins, which can easily lead to some of them breaking. Is there a specific reason why we're doing this? If it's a behavior left over from something else, I can remove it.

Your environment

  • Name and version of operating system: macOS 10.14.4
  • Names and version of required packages: 1.0.18
  • Version of node, npm, yarn, or names and versions of browser: yarn 1.16.0

Steps to reproduce

import mdx from '@mdx-js/mdx'
import detectFrontmatter from 'remark-frontmatter'

const content = `
---
title: Hello World!
---
`

mdx(content, {
  remarkPlugins: [
    detectFrontmatter,
  ],
})

Expected behaviour

remark-frontmatter shouldn't be getting unexpected options.

Actual behaviour

Options from the mdx() call get forwarded to remark-frontmatter, which thinks we're passing the definition for detecting frontmatter, but the shape of the object doesn't satisfy its criteria so it crashes.

Temporary workaround

Passing undefined as an option to all plugins, which results in default, i.e. expected behavior.

mdx(content, {
  remarkPlugins: [
    [detectFrontmatter, undefined],
  ],
})
馃悰 typbug

Most helpful comment

I suspect there is a teeny tiny extremely slim chance someone depends and we鈥檇 break something if we patch it up. If we do a major (or wait for one) the chance of someone being on ^1.0.0 and trying to use a plugin that breaks is much much much larger.

I also think @silvenon has a good enough understanding of remark and MDX that he can figure out what happened rather easily, but I don鈥檛 think many other persons using MDX can figure out what鈥檚 failing, resulting in a pretty bad debugging experience.

Therefore I propose patching this up instead of a major release. We can add a note about this in releases though.

All 6 comments

Funky, yeah I鈥檓 wondering what the reason is. Seems to be there since the beginning: 587477d

Yeah, I doubt any reason can justify this behavior, so I'll prepare a PR just in case.

I鈥檓 pretty sure a PR is good, but wondering what @johno thinks.

This also could be, very very theoretically, breaking. But it鈥檚 much more likely that it breaks plugins like you describe. So I鈥檇 say this shouldn鈥檛 need a major semver release.

Yeah this is really an artifact of a quick hack that was never cleaned up. I _think_ this is okay without a major since it's likely breaking plugins, but not 100% sure there.

I suspect there is a teeny tiny extremely slim chance someone depends and we鈥檇 break something if we patch it up. If we do a major (or wait for one) the chance of someone being on ^1.0.0 and trying to use a plugin that breaks is much much much larger.

I also think @silvenon has a good enough understanding of remark and MDX that he can figure out what happened rather easily, but I don鈥檛 think many other persons using MDX can figure out what鈥檚 failing, resulting in a pretty bad debugging experience.

Therefore I propose patching this up instead of a major release. We can add a note about this in releases though.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

slorber picture slorber  路  19Comments

johno picture johno  路  53Comments

khrome83 picture khrome83  路  14Comments

naivefun picture naivefun  路  12Comments

brennj picture brennj  路  14Comments