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

[Discussion] Add JSX support to simplify render function components and Vue 2 + Vue 3 support #6024

Open
ShGKme opened this issue Aug 29, 2024 · 6 comments
Assignees
Labels
discussion Need advices, opinions or ideas on this topic

Comments

@ShGKme
Copy link
Contributor

ShGKme commented Aug 29, 2024

The proposal is not about migration <template> -> JSX but from render: () => h(tag) to render: () => <tag /> in the components already implemented via render functions.

Adding JSX support allows:

  1. Simplify complex render functions
  2. Decrease the difference between Vue 2 and Vue 3 version (render function API changed a lot)
  3. Improve performance in Vue 3 with complite-time optimizations

Required steps:

  • Add Vite plugin for JSX
  • Configure webpack for vue-styleguidist
  • Rewrite components NcActions, NcButton
@ShGKme ShGKme added the discussion Need advices, opinions or ideas on this topic label Aug 29, 2024
@ShGKme ShGKme self-assigned this Aug 29, 2024
@susnux
Copy link
Contributor

susnux commented Aug 29, 2024

It depends on what exactly you mean.
If you just mean get rid of render functions in favor of SFC (template + script + styles) I fully agree.

But if you mean real JSX, like bringing React syntax¹, then I would vote against this, as it brings a new syntax that adds a new layer of complexity and is very uncommon across the Nextcloud ecosystem.
E.g. we also consider migrating groupfolders from React to Vue as JSX makes maintenance a lot harder (not only the framework but the syntax makes quick fixes not really quick).


I do not think that it is so important to do anything here for the Vue 2 vs Vue 3 part².
But I agree that render functions are more hard to maintain, as you need to get into that code - compared to a simple template. So I would propose to just migrate to templates.

For NcButton there is no good reason to not use template + script, but actions are a bit more trickier. This is unrelated but I would even propose that we refactor actions into two components: A simple popover menu without inline button support - this will allow custom components for grouping. And NcActions that allow inline actions, this would also made responsive and move actions into the popover menu when space is limited.


¹ with React style I mean JSX like:

export default {
    ....
  methods: {
      checkStatement(){
        if (this.user.age > 18) {
           return <div> Welcome, { this.user.name }</div>;
        }
      }
    },
    ...
}

²As I am still in hope that we migrate everything soon ( 🤞 ) and can bury Vue 2 soon (so only need to provide bug fixes, like with v7 currently). There is no (unpaid) support for Vue2 anymore, its really time to push this.

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 30, 2024

It depends on what exactly you mean.

What title says - use JSX syntax for render-function.

BeforeAfter
// Before

// Vue 2
export default {
  render() {
    if (this.to) {
      return h('router-link', {
        props: {
          custom: true,
            to: this.to,
            exact: this.exact,
          },
          scopedSlots: {
            default: renderButton,
          },
        })
      }
    // Otherwise we simply return the button
    return renderButton()
  }
}
// After (Vue 2 + Vue 3)
export default {
  render() {
    if (this.to) {
      return <RouterLink custom to={this.to}>{
        default: renderButton
      }</RounterLink>
    }
    // Otherwise we simply return the button
    return renderButton()
  }
}
// Vue 3
export default {
  render() {
    if (this.to) {
      return h(resolveComponent('router-link'), {
          custom: true,
          to: this.to,
        }, {
	  default: renderButton,
      })
    }
    // Otherwise we simply return the button
    return renderButton()
  }
}
Same

Benefits:

  1. Simplicity
  2. Hide the difference between Vue 2 and Vue 3 render function API
  3. Performance in Vue 3 with all the optimizations we loose writing render functions manually

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 30, 2024

if you mean real JSX, like bringing React syntax

But this is not a "React syntax". JSX is a commonly used JavaScript syntax sugar. It is used by many frameworks, including modern like Solid.js, MDX syntax, and Vue.

If you mean an approach - using render function instead of template - we use it already. JSX doesn't change this fact, it only makes it less painful and better in performance.

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 30, 2024

If you just mean get rid of render functions in favor of SFC (template + script + styles) I fully agree.

I have no idea how to do it in NcActions without a huge breaking change and/or performance regression. Keeping in mind that it is used in many places and is a part of other components API.

And in NcButton without much increase complexity or duplications.

We can think about it, but this is something for future with huge changes in many places.

Currently I only propose to makes the current state simpler and more performant.

@ShGKme
Copy link
Contributor Author

ShGKme commented Aug 30, 2024

E.g. we also consider migrating groupfolders from React to Vue as JSX makes maintenance a lot harder (not only the framework but the syntax makes quick fixes not really quick).

I don't think raw render-function API is simpler. It's even harder. And it looses compile-time optimizations. And you have to learn 2 APIs for Vue 2 and Vue 3.

Learning JSX itself is not a big deal. If you already know render function api, then learning JSX is learning sugar with just 2 things.

image

@susnux
Copy link
Contributor

susnux commented Aug 30, 2024

And in NcButton without much increase complexity or duplications.

It depends on what you consider complexity. The problem there is the router link.
Without that it can be just written as one template, nothing special and very easy to understand.

But because you need to conditionally wrap it inside a router-link you need to either duplicate it or split it in two (three) components.
So either:

<RouterLink v-if="isRouterLink">
    <ButtonImplementation />
</RouterLink>`
<ButtonImplementation v-else />

or add a third (empty) dummy component

<component :is="isRouterLink ? 'router-link' : EmptyDummy">
    <ButtonImplementation />
</component>`

I don't think raw render-function API is simpler. It's even harder.

Yes on that I agree :)

Learning JSX itself is not a big deal. If you already know render function api, then learning JSX is learning sugar with just 2 things.

But it is not used, so it is new and only required for this place. So you expect people to learn it for two places, that can (more or less) easily just rewritten as templates.
I do not think learning is a problem, but the use case is very limited and basically not a requirement currently.
This will make understanding it harder. e.g. suddently < inside code is not smaller-than but an HTML tag.


Some time ago I started doing exactly that: Migrate NcButton to a template, now you motivated me to finish that 😅
You can find the example here: #6033

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Need advices, opinions or ideas on this topic
Projects
None yet
Development

No branches or pull requests

2 participants