Vue-kitchen-sink: Re-enable MDX storyshots

[shilman]

Issue: #7223

What I did

• Re-enable MDX storyshots in examples/vue-kitchen-sink
• Upgrade vue-kitchen-sink to latest (in an attempt to fix babel incompat, see below)

What I need

@Aaron-Pool

• verify upgrade
• debug MDX snapshot error if possible, see Addon-docs: Jest MDX transform for storyshots #8189 for context

$ yarn jest --testPathPattern=vueshots.test.js

...

 FAIL  examples/vue-kitchen-sink/vueshots.test.js
  ● Test suite failed to run

    This API has been removed. If you're looking for this functionality in Babel 7, you should import the '@babel/helper-module-imports' module and use the functions exposed  from that module, such as 'addNamed' or 'addDefault'.

      at File.addImport (node_modules/@babel/core/lib/transformation/file/file.js:175:11)
      at PluginPass.addImport (node_modules/@babel/core/lib/transformation/plugin-pass.js:35:22)
      at buildOpeningElementAttributes (node_modules/babel-plugin-transform-vue-jsx/index.js:193:25)
      at buildElementCall (node_modules/babel-plugin-transform-vue-jsx/index.js:130:17)
      at PluginPass.exit (node_modules/babel-plugin-transform-vue-jsx/index.js:30:26)
      at newFn (node_modules/@babel/traverse/lib/visitors.js:195:21)
      at NodePath._call (node_modules/@babel/traverse/lib/path/context.js:53:20)
      at NodePath.call (node_modules/@babel/traverse/lib/path/context.js:40:17)
      at NodePath.visit (node_modules/@babel/traverse/lib/path/context.js:97:8)
      at TraversalContext.visitQueue (node_modules/@babel/traverse/lib/context.js:120:16)

[vercel]

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/storybook/monorepo/g223ipfjt
🌍 Preview: https://monorepo-git-7223-mdx-snapshot-vue.storybook.now.sh

[github-actions]

	Fails
🚫	PR is marked with "do not merge", "in progress" labels.

Generated by 🚫 dangerJS against 78b29da

[Aaron-Pool]

@shilman Taken me a while to get back around to this, and I'm not sure where exactly we left things, so I just wanted to do a quick summary of the source of the bug and explain the different options for dealing with this issue, and maybe get some direction from the core maintainers.

The Bug:

The root cause of the bug is that, by default, the babel-preset-vue preset includes a plugin called babel-plugin-jsx-v-model (see here). This plugin is incompatible with babel 7(see here) and results in the bug that gets thrown.

Possible Solutions:

The solutions here need to be considered more from the perspective of @storybook/vue as a whole moving to use Babel 7, because there are other places (documentation, presets, etc) that explicitly reference or utilize babel-preset-vue and these need to be addressed as well.

1) Disable babel-plugin-jsx-v-model in the babel-preset-vue configuration

like so:

{
  "presets": [
    ["vue", {
      "vModel": false
    }]
  ]
}

This is definitely a breaking change. Anyone who happens to be using v-model syntactic sugar in their jsx for storybook vue will no longer be able to build their storybook after upgrading, until they refactor v-model out of their codebase.

2) Upgrade to @vue/babel-preset-jsx + @babel/preset-env

Vue has made significant strides in the past two years to standardize and make their jsx support more "official". There is now a @vue/jsx monorepo under the vue organization that has all the packages needed to transpile vue-flavored jsx, all of which are included in the provided preset. That combined with the traditional @babel/preset-env creates equivalent transpilation behavior to what is currently being done by babel-preset-vue, but, of course, has full support for babel 7.

This option may not be a breaking change. As far as I can tell, the only syntax changes between the outdated preset and the new, official one are additive. The only exception could be that the dynamic h injection method may be more reliable now, meaning some users could possibly get duplicate 'h' declaration errors in places where the plugin previously failed to automatically inject h and they were forced to manually inject h. But the evidence for the possibility is basically a foggy memory I have of maybe running into that issue when I upgraded to the new preset.

3) Do nothing

I feel like this will probably create it's own issues. But you can always punt the issue for now, depending on how important vue mdx storyshot functionality is.

---

So, that's where this issue is at. I'm glad to go down either of those roads, but since there was even a remote possibility of a breaking change, I wanted to layout the possible solutions and get some input from the core maintainers before going too far down any particular route.

Disclaimer: I wrote this at around 2 in the morning when I coudn't seem to fall asleep, if there are typos, or things that don't make sense... Well, I'm blaming it on that.

[shilman]

You rock @Aaron-Pool. We should definitely go with option 2, but the question is whether we do this now, in 5.4, or in 6.0?

It boils down to:

• Priority: How important is snapshot testing for Vue MDX? In my opinion this is nice to have but something we can live without.
• Semver: How strict do we want to be about semver? If strict, we should hold off to 6.0. If we're OK with minor deviations that come from switching libraries, which I am, then we can do it sooner.
• Release schedule: If we decided to pull the trigger, could you get it done by Nov 11 (5.3 beta) if you wanted to? Feels tight to me.

Given all this, I'd say let's punt this until 5.4 at earliest. Unless you think you can still pull off the babel-macro stuff and this helps that.

Cc @elevatebart @backbone87 @pksunkara @ndelangen @Hypnosphi @tmeasday @igor-dv

[piemasters]

Hey, just wanted to check if there's been any update on this, or if there's a good way for storyshots to ignore .mdx files for now?

Not having snapshots for mdx stories for now isn't the end of the world, but it would be good to skip them to avoid the errors. I've tried both storyKindRegex and storyNameRegex patterns to avoid them but it doesn't seem to work for mdx files.

[shilman]

@piemasters this is the hack I used to disable MDX until we fix this:

https://github.com/storybookjs/storybook/pull/8557/files#diff-965ae81ff37aac26ce1cd613a1bb8c37L11-L15