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

CLI: MDX template support #8396

Merged
merged 38 commits into from Oct 14, 2019
Merged

CLI: MDX template support #8396

merged 38 commits into from Oct 14, 2019

Conversation

shilman
Copy link
Member

@shilman shilman commented Oct 12, 2019

Issue: #7543

What I did

  • Added --story-format option to sb init, defaulting to CSF
  • Added MDX template support for React, Vue, Angular, HTML
  • Prepared all templates for multiple story format support (e.g. typescript)
  • Updated the CLI tests for MDX

How to test

New test argument for testing MDX:

cd lib/cli
yarn test -s mdx

@vercel vercel bot temporarily deployed to staging October 12, 2019 03:38 Inactive
@shilman shilman added this to the 5.3.0 milestone Oct 12, 2019
@shilman
Copy link
Member Author

shilman commented Oct 13, 2019

@Aaron-Pool I've tested all of the templates per the instructions above, and the Vue MDX templates are failing due to some babel issue. Would you mind taking a look? The difference between these test cases and examples/vue-kitchen-sink is not obvious to me on first look.

@shilman
Copy link
Member Author

shilman commented Oct 13, 2019

@tmeasday @igor-dv The React examples are mostly failing unless I set configureJSX = true. I'm wondering whether it should be true by default starting in 6.0, or whether we can do some more sophisticated auto-detection? This seems to be the last obstacle to a nice zero-config setup for React.

@Aaron-Pool
Copy link
Contributor

@shilman didn't have time to dig in super deep this morning, but I did see that you have two Vue mdx stories with JSX, and to the best of my knowledge, that's still a no go.

@tmeasday
Copy link
Member

@shilman I don't really understand what's going on there you may need to spell it out for me.

@shilman
Copy link
Member Author

shilman commented Oct 14, 2019

@Aaron-Pool the MDX files in the template are functions that return objects that contain template: '<some jsx>' fields, which is the same as what's in vue-kitchen-sink. AFAIK it's "inline" JSX that's still broken for vue, no?

@Aaron-Pool
Copy link
Contributor

Aaron-Pool commented Oct 14, 2019

@shilman, I don't think that's correct. If you look at the areas I commented, they use JSX in render functions. Which aren't template: '<blah/> format. The core technical issue with Vue jsx in the MDX doc is that it introduces a second distinct flavor of jsx on the page. Both flavors need to be transpiled by their own loader, and only their own loader, or else the result won't be correct.

So, no matter what order the Babel loaders run in, the final product be in an error state. If the Vue jsx one runs first, then it will incorrectly transpile the jsx outside of the story blocks and things will be broken. If react jsx runs first, it will try to transpile the jsx in the story incorrectly, and things will be broken. Bit of a catch-22.

That was why in a thread a while ago I suggested using babel macros by Kent Dodds to implement story blocks for different view layers, because it's a pretty elegant way of scoping Babel transformations to a particular code block. Still something worth considering imho.

@shilman
Copy link
Member Author

shilman commented Oct 14, 2019

@Aaron-Pool You're right! 🙇 I'm going to remove that story from the template for now, and we can figure a better solution together separately.

UPDATE: Removed the JSX stories but still getting the same error. 😭 What else is different about the vue-kitchen-sink example that it works?

Copy link
Contributor

@Aaron-Pool Aaron-Pool left a comment

Choose a reason for hiding this comment

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

Looks fine for me, in the Vue area, at least 👍

@shilman shilman merged commit 4fcdee3 into next Oct 14, 2019
@shilman shilman deleted the 7543-cli-mdx branch October 14, 2019 16:34
@shilman shilman mentioned this pull request Oct 14, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants