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

Render function API change #28

Merged
merged 7 commits into from
Nov 12, 2019
Merged

Render function API change #28

merged 7 commits into from
Nov 12, 2019

Conversation

yyx990803
Copy link
Member

@yyx990803 yyx990803 commented Apr 8, 2019

@yyx990803 yyx990803 added breaking change This RFC contains breaking changes or deprecations of old API. core 3.x This RFC only targets 3.0 and above labels Apr 8, 2019
@jacekkarczmarczyk
Copy link

render(
    // declared props
    props,
    // resolved slots
    slots,
    // fallthrough attributes
    attrs,
    // the raw vnode in parent scope representing this component
    vnode
  ) {}

Is there a reason for passing 4 arguments instead of 1 object which dev could destruct?

render({ slots, vnode}) => ...

@HerringtonDarkholme
Copy link
Member

@jacekkarczmarczyk One perk of positional arguments is easier type annotation. For object style argument, users have to annotate all property types. Positional arguments can be optionally skipped and only what users want to specify need be annotated.

@dimensi
Copy link

dimensi commented Apr 8, 2019

@jacekkarczmarczyk One perk of positional arguments is easier type annotation. For object style argument, users have to annotate all property types. Positional arguments can be optionally skipped and only what users want to specify need be annotated.

And how you skip position arguments?

@posva
Copy link
Member

posva commented Apr 8, 2019

I think it's more about not allocating an extra object when rendering every component

@Kelin2025
Copy link

Kelin2025 commented Apr 8, 2019

render({ propA, propB }) { /* ... */ }
vs
render({ props: { propA, propB } })

I guess the choice is obvious

@yyx990803
Copy link
Member Author

yyx990803 commented Apr 9, 2019

@jacekkarczmarczyk this is a carefully considered choice. These arguments are ordered from more commonly used to less commonly used. Most users will probably never use the 4th argument (vnode), whereas code that uses only one or two arguments will be the most common:

render(props) { /* ... */ }
render(props, slots) { /* ... */ }

In these cases the perks mentioned by @HerringtonDarkholme and @Kelin2025 become more obvious. Plus there's the small perf benefits of avoiding an extra object and the destructuring.

@yyx990803 yyx990803 changed the title Render function api change Render function API change Apr 9, 2019
@leopiccionia
Copy link

I'd like to present a point against the {on: {click: foo}} to {onClick: foo} migration.

HTML event names are (AFAIK) casa-sensitive, and the automatic case conversion can affect Vue's perfect score on Custom Elements Everywhere benchmark. For example, what's the equivalent to {on: {Click: foo}} in current API? Perhaps an escape hatch is needed.

@znck

This comment has been minimized.

@yyx990803
Copy link
Member Author

yyx990803 commented Apr 11, 2019

@leopiccionia that's a good point - we actually need to consider this for component custom events as well. Maybe we can use the on: prefix to indicate that the event name should be used verbatim:

h('custom-element', {
  // listens to "someevent"
  onSomeEvent: e => {
  },
  // listenes to "SomeEvent"
  'on:SomeEvent': e => {
    // ...
  }
})

@rashfael
Copy link

  • for anything else:
    • If the key exists as a property on the DOM node, it is set as a DOM property;
    • Otherwise it is set as an attribute.

Does that mean that we cannot add our own custom properties to a DOM node anymore, like vue does with __vue__?

We are currently using:

domProps: {
  __ourSoftware__: {…}
}
´´´
to add references for use in event handlers.

@yyx990803
Copy link
Member Author

@rashfael interesting, I guess we need to keep escape hatches for all explicit binding types.

How about:

h('div', {
  'attr:id': 'foo',
  'prop:__ourSoftware__': { ... },
  'on:click': () => { ... }
})

@znck
Copy link
Member

znck commented Apr 12, 2019

How to use escape hatches in template syntax?

@petr001
Copy link
Contributor

petr001 commented Apr 12, 2019

@znck The same as before? And for DOM props with even easier syntax: https://github.com/vuejs/rfcs/blob/v-bind-prop-shorthand/active-rfcs/0000-v-bind-prop-shorthand.md

@LinusBorg
Copy link
Member

LinusBorg commented Apr 12, 2019

@yyx990803

I guess we need to keep escape hatches for all explicit binding types.

We should carefully evaluate the possible performance impact of having to match each prop name of each and every vnode against 3 RegExp's (or something similar), when these seemt to be edge cases.

Techncially we can use custom directives to apply these things as well, right? Not nearly as ergonomic, of course ...

@JosephSilber
Copy link

@LinusBorg does it really have to run those regex patterns for every prop?

It'd be faster to check if the string has a colon with a simple .includes(':'), then split on the colon, then compare to a set of prefixes.

This way there's a very minimal cost to props not using a colon.

@LinusBorg
Copy link
Member

LinusBorg commented Apr 12, 2019

Of course it doesn't have to be a regexp, which is why I wrote

(or something similar)

To be honest I have no good intuition about the cost of these operations one way or another, it just seems wasteful to do this check for literally thousands and thousands of attributes in your app's virtual dom given we will likely have very few occurrences of these escape hatches.

@leopiccionia
Copy link

leopiccionia commented Apr 12, 2019

Techncially we can use custom directives to apply these things as well, right? Not nearly as ergonomic, of course ...

@LinusBorg Do you mean something like v-attr:id and v-prop:__ourSoftware__? It doesn't look bad for an edge case IMHO, and could be either tree-shaken or implemented in userland.

I don't know how custom directives are handled in diff algorithm. Would there be a performance penalty (in comparison to something like :id.attr and :__ourSoftware.prop)?

@leopiccionia
Copy link

leopiccionia commented Jun 11, 2019

@ycmjason It's an older proposal, that actually passed four arguments, instead of an object parameter.

This proposal is very elegant, but, as Evan's said here, may be reverted to passing an object if Vue decides to introduce more arguments.

@darrenjennings
Copy link

does this feature imply that you could render jsx inside templates? 😅
vuejs/vue#7439

<template>
  <div>
    {{ h('h1', 'hello') }}
    {{ <h1>hello</h1> }} <!-- would `h` be accessible here? -->
  </div>
</template>

<script>
import { createElement as h } from 'vue'

export default {
  data () { return { h } }
}
</script>

@sustained
Copy link

sustained commented Sep 2, 2019

This is nice but one small gripe -

Personally I think this escape hatch API more sense:

h('div', {
  'attr:id': 'foo',
  'prop:__ourSoftware__': { ... },
  'event:click': () => { ... }
})

I.e.:

-   'on:click': () => { ... }
+   'event:click': () => { ... }

...despite that we use v-on in templates.

But I'm sure many will disagree.

My reasoning is that the first two describe the what.

  • attr: is for attributes
  • prop: is for props
  • on: is for ons - oh wait, that doesn't work

See what I mean?


The counter-argument will be that the word on is so heavily tied to events in our minds that this is not an issue. And perhaps you're right.. still.

@yyx990803 yyx990803 added the final comments This RFC is in final comments period label Nov 6, 2019
@yyx990803
Copy link
Member Author

This RFC is now in final comments stage. An RFC in final comments stage means that:

  • The core team has reviewed the feedback and reached consensus about the general direction of the RFC and believe that this RFC is a worthwhile addition to the framework.
  • Final comments stage does not mean the RFC's design details are final - we may still tweak the details as we implement it and discover new technical insights or constraints. It may even be further adjusted based on user feedback after it lands in an alpha/beta release.
  • If no major objections with solid supporting arguments have been presented after a week, the RFC will be merged and become an active RFC.

@tobyzerner
Copy link

I don't have a good intuition about the performance impact of checking for the escape hatch prefixes either... but as a cheaper alternative perhaps these properties could be wrapped in an object, so most of the time the algorithm would only have to check for the existence of a single key:

h('div', {
  id: 'foo',
  explicit: {
    'attr:something': 'bar',
    'prop:__ourSoftware__': { ... },
    'on:Click': () => { ... }
  }
})

@LinusBorg
Copy link
Member

@tobyzerner that's a good idea I think.

We could use a Symbol as the key so we don't have to add another reserved prop name besides key and ref

@yyx990803 yyx990803 merged commit 5fb76aa into master Nov 12, 2019
@lbogdan
Copy link

lbogdan commented Jan 5, 2020

@yyx990803 There's a typo in the link in your first comment, should be 0008-render-... instead of 0000-render-....

@yyx990803 yyx990803 added feat: render function Only affects render function usage - can be ignored if only using templates and removed final comments This RFC is in final comments period labels Jul 1, 2020
@bencodezen bencodezen mentioned this pull request Jul 6, 2020
25 tasks
@shrpne
Copy link

shrpne commented Jul 28, 2020

Can h still be passed to render function as an argument?

It's pretty important for me, as a plugin author.
Current global import approach requires a lot of overhead during development:

  • tweak build to exclude vue-core from bundle (easy)
  • add local dev server with node_modules handling (not easy), previously I just imported my component's source as browser's es-module, now browser's es-module can't resolve vue as it is node_module and not a real path
  • plugin users sometimes may experience issues with conflicts of vue core, when their build process is not properly configured (rare)

I understand h now is not context-specific but what prevents us to pass it as an argument?
It should take no costs, as any component needs h to somehow render VNodes. So h will be in the app bundle anyways.
And it will eliminate current drawback of Reliance on Vue Core
So there will be two ways to access h: new global import and old one. And everyone should be happy :)

@jackchoumine
Copy link

@jacekkarczmarczyk One perk of positional arguments is easier type annotation. For object style argument, users have to annotate all property types. Positional arguments can be optionally skipped and only what users want to specify need be annotated.

And how you skip position arguments?

like this:

// skip props
render(_,slots){
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x This RFC only targets 3.0 and above breaking change This RFC contains breaking changes or deprecations of old API. core feat: render function Only affects render function usage - can be ignored if only using templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.