Skip to content
This repository has been archived by the owner on Mar 8, 2019. It is now read-only.

feat: add slot support in jsx render function #101

Merged
merged 8 commits into from
Feb 8, 2019

Conversation

elevatebart
Copy link
Member

fix #100

Copy link

@b12f b12f left a comment

Choose a reason for hiding this comment

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

Looks good! Props for the quick work.

const renderValuePath = renderPath[0].get('value')
const renderValuePath = bt.isObjectProperty(renderPath[0].node)
? renderPath[0].get('value')
: renderPath[0]
recast.visit(renderValuePath, {
Copy link

Choose a reason for hiding this comment

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

Since you can return JSX from both methods and computed properties (and maybe even data?), I think we should visit the full Vue object here.

Copy link

Choose a reason for hiding this comment

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

It'll make the PR bigger though since we need to add all the relevant tests

Copy link
Member Author

Choose a reason for hiding this comment

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

that is a good idea. In this case, you should create another handler so that not to polute the render function handler already present.

@@ -95,6 +95,8 @@ export default {
const { sortKey, capitalize } = this
return (
<table class="grid">
{/** @slot Use this slot header */}
<slot name="header" />
Copy link

Choose a reason for hiding this comment

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

Perhaps also add a method/computed property with the footer slot, and then include it below. See my comment above

src/script-handlers/slotHandler.ts Outdated Show resolved Hide resolved
src/script-handlers/slotHandler.ts Outdated Show resolved Hide resolved
src/script-handlers/slotHandler.ts Outdated Show resolved Hide resolved
src/script-handlers/slotHandler.ts Outdated Show resolved Hide resolved
@elevatebart
Copy link
Member Author

Hello @b12f,

You review is very appreciated. :-)
I think all your remarks are valid. I will fix some but not necessarily run the parser on the whole file.
Do you wish me to merge this as is? You could then pull and create another PR?

@elevatebart elevatebart self-assigned this Feb 8, 2019
@b12f
Copy link

b12f commented Feb 8, 2019

Feel free to merge, I'll look at how we might be able to do full component parsing.

Copy link

@b12f b12f left a comment

Choose a reason for hiding this comment

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

Another way to do the comment expression find is functionally: slice > reverse > find.

@elevatebart elevatebart merged commit 8ad6f95 into master Feb 8, 2019
@elevatebart elevatebart deleted the feat-jsxslotsupport branch February 8, 2019 14:30
@vue-styleguidist-bot
Copy link
Collaborator

🎉 This PR is included in version 3.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSX slot support
3 participants