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

Class API #4

Closed
yyx990803 opened this issue Oct 9, 2018 · 42 comments
Closed

Class API #4

yyx990803 opened this issue Oct 9, 2018 · 42 comments

Comments

@yyx990803
Copy link
Member

yyx990803 commented Oct 9, 2018

This is a reference of the current implemented version and open to discussion.

Update: don't know since when but latest Chrome Canary now enables class fields by default, so we can already play with the following API without a transpiler.

Plain ES usage:

import { h, Component } from '@vue/renderer-dom'

class App extends Component {
  static props = {
    msg: String
  }

  // data
  count = 0

  // lifecycle
  created() {
    console.log(this.count)
  }

  // getters are converted to computed properties
  get plusOne() {
    return this.count + 1
  }
  
  // a method
  increment() {
    this.count++
  }

  render(props) {
    return h('div', [
      h('span', this.count),
      h('span', props.msg,
      h('button', {
        // methods are auto-bound when they are accessed via the render proxy
        onClick: this.increment
      }, 'increment')
    ])
  }
}

TS Usage:

import { h, Component, ComponentWatchOptions } from '@vue/renderer-dom'

interface Props {
  msg: string
}

interface Data {
  count: number
}

class App extends Component<Props, Data> {
  static props = {
    msg: String
  }
  
  // data fields
  count: number = 0

  // ComponentWatchOptions type is only needed if `this` inference
  // is needed inside options, e.g. in watch callbacks
  static watch: ComponentWatchOptions<App> {
    count(value) {
      console.log(value)
    }
  }

  created() {
    console.log(this.count)
  }

  get plusOne() {
    return this.count + 1
  }

  increment() {
    this.count++
  }

  render(props) {
    // ...
  }
}
@znck
Copy link
Member

znck commented Oct 9, 2018

import { h, Component } from '@vue/renderer-dom'

Doesn't using h and Component from '@vue/renderer-dom' couples component to one renderer?

@phanan
Copy link
Member

phanan commented Oct 9, 2018

open question: should we rename them to onXxx to differentiate from methods?

I'm all for it. Even for v2.0 and below (where methods are contained within their own object obviously), I always find myself itching for the convention. Another approach might be to use the denoting $ (though I'm not leaning toward this because it looks a tad ugly).

EDIT: But then again, users can add onXxx methods on their own e.g. event handlers, so on on its own (heh, pun intended) might not cut it.

@yyx990803
Copy link
Member Author

@znck h is just re-exported from @vue/core for convenience. It works regardless of render target. You can also import it from @vue/core.

@phanan
Copy link
Member

phanan commented Oct 9, 2018

// getters are converted to computed properties

While it's convenient (and I like the syntax), is it always the expected behavior?

@yyx990803
Copy link
Member Author

@phanan as long as we document this.

@kazupon
Copy link
Member

kazupon commented Oct 9, 2018

open question: should we rename them to onXxx to differentiate from methods?

IMO, we should not change the lifecycle method names in v3, because of we should be avoided migration fatigue to v3 from v2 as possible, however, as official blog mentioned, we will plan to support about the compatible features for 2.x, so I think It maybe not much of the problem.

Additionally If we will be able to support the migration tool, it's wonderful.

@phanan
Copy link
Member

phanan commented Oct 9, 2018

because of we should be avoided migration fatigue to v3 from v2 as possible, however, as official blog mentioned, we will plan to support about the compatible features for 2.x

While I hear this, we might as well keep in mind that these kinds of backward API changes can only be introduced with major versions, which means if we don't take this opportunity, we'll have to wait for v4 (another 2 years perhaps?) 😄

@chrisvfritz
Copy link
Contributor

static options: ComponentOptions<App> = {

To me, it looks very hacky to keep some options top-level and some under an options field. Is there a way we can keep everything top-level? E.g.:

static props: ComponentProps<App> = {
  // ...
}

App.options = options

This pains me. 😅 Since we're relying on a stabilized class API for Vue 3, why can't we use static field declarations for the non-TS version as well? E.g.:

static props = {
  // ...
}

props: {
msg: String
}

This probably isn't possible or would come with too many edge cases, but in TS, I think it'd be great if we could avoid defining types twice for each prop. Is there possibly some way (e.g. with a fancy TS compiler option) that we could access TS type info at runtime in development?

should we rename them to onXxx to differentiate from methods?

I agree with @kazupon in this instance. We already have a nice convention for lifecycle methods that differentiate them from methods: either past tense verb, or before + imperative verb. Changing to onXxx would increase confusion I think, since I currently see a lot of methods use that convention (e.g. onRowHover, onButtonClick) - even though I personally find it to be an anti-pattern in most cases (I think we should describe what we're doing in the method name, not when we happen to call it).

we might as well keep in mind that these kinds of backward API changes can only be introduced with major versions

@phanan I agree with this, but I still think there has to be significant benefit if we're going to force users to relearn something.

@Jinjiang
Copy link
Member

Jinjiang commented Oct 9, 2018

we might as well keep in mind that these kinds of backward API changes can only be introduced with major versions

The naming difference could be adapted through createComponentClassFromOptions(options) in https://github.com/vuejs/vue-next/blob/ba62deb5d938c43c41ec5e7686dc006a91d809b8/packages/core/src/componentUtils.ts#L170 I guess, fine for v2 component options.

I think it'd be great if we could avoid defining types twice for each prop.

Agree.

@yyx990803
Copy link
Member Author

@chrisvfritz static class fields are not in the spec yet. Currently in plain ES you can only do static getters.

Is there a way we can keep everything top-level?

Maybe. Having the options as an object is easier for compatibility reasons. Making them flat involves extracting these flat properties back into an object for easier reflection.

I think it'd be great if we could avoid defining types twice for each prop.

This is unlikely because the static types and the options serve slightly different purposes:

  1. The options performs runtime checking which is more meaningful for component consumers instead of authors. It also can contain things like required: true, custom validators and default values, which cannot be expressed in the interface.

  2. On the other hand, the interface allows for more intricate typing constraints (e.g. object shapes) that cannot be easily expressed through the options.

@yyx990803
Copy link
Member Author

Edit: changed options to be flat static properties on the class

@HerringtonDarkholme
Copy link
Member

open question: should we rename them to onXxx to differentiate from methods?

IMO, we should not change the lifecycle method names in v3, because of we should be avoided migration fatigue to v3 from v2 as possible.

From the perspective of vue-class-component, builtin lifecycle names haven't caused many problems or confusion. On the other hand onXXX as @chrisvfritz has said is used widely by users.


Computed properties can also have a lazy option. But it looks like Vue 3 will make computed lazy by default. https://github.com/vuejs/vue-next/blob/bf38fea31388dd8c3a2f4206221089a2ed92e841/packages/observer/src/computed.ts#L9
So we don't need the lazy option any, do we?

@chrisvfritz
Copy link
Contributor

static class fields are not in the spec yet. Currently in plain ES you can only do static getters.

@yyx990803 I might have misunderstood, but I thought we were waiting to release Vue 3 until class features had stabilized. Field declarations and static class fields are both stage 3 right now, so I was assuming we'd be able to rely on them by the time of release.

To me, field declarations (both static and instance-based) are an absolutely essential feature of any class API from a DX perspective - and for the record, these are the only existing class proposals I feel that way about. Beyond breaking the logical options order we've recommended for readable components, tacking on extra properties after the initial definition is hacky enough that I think it'd become impossible to sell classes as an improvement.

We're already going to get a lot of pushback from people who prefer the object-based API. It's so simple and well-organized that outside of TS, it's rare to hear anything but praise for it. That means probably most users will feel threatened initially, because they'll see classes as a step towards deprecating the object-based API they love (and their assumption is probably accurate). For that reason, I feel like the experience has to be at least as good with classes.

@yyx990803
Copy link
Member Author

yyx990803 commented Oct 10, 2018

@chrisvfritz

Ideally, we want to wait until class fields become part of the spec before releasing 3.0. I'm thinking somewhere around mid 2019, and with them being currently stage 3, it's unlikely for them to make it into ES2019. However, it's hopeful that they are stage 4 around that point, and that would be safe enough to recommend usage via transpilers (which most serious users will still need, for the time being). If we've reached a day where most users are shipping non-transpiled code, I'm pretty sure class fields would have landed by then.

Also, we can actually make static getters work:

class Foo extends Component {
  static get props() {
    return {
      msg: String
    }
  }
}

Finally, we are not going to deprecate object-based syntax in 3.x, those who prefer it can keep using it. As I said, this is the less "feeling threatened" way to look at it: Components have always been internally represented as classes, even in 2.x (as function constructors). We are just converting options objects into classes. However, in 2.x there's no way to directly author this internal class using the class syntax. 3.0 provides a way to do that, while still keeping the option to use objects.

Note that while there are users who strongly prefer the object syntax, there are other users who strongly prefer the class-based syntax too (especially for users coming from languages where class is the norm). Shying away from classes essentially sacrifices the DX for the class-preferring group where the only benefit is making the object-preferring group feel "comfortable" or "safe".

@chrisvfritz
Copy link
Contributor

@yyx990803 As I understand it, one of the main reasons we're defaulting to a class-based API is to prevent the schism where TypeScript users have to use a significantly different interface from everyone else. I agree that's a problem worth solving.

However, if we're unable to sell the benefits of the class-based API to a large group of users, we'll actually have failed to erase that schism. Instead, I think we'll actually have made it worse, because then the cognitive burden will have shifted from some of our most advanced users (TypeScript users) to our least advanced users (those who either feel intimidated by classes or have bad experiences with class components early on). Being the least advanced, they'll have a much, much harder time bridging the gap and translating examples - especially since unlike current TypeScript users, they'll actually have to learn new, unfamiliar language features to do so.

This is why I feel that all major class features that affect component definitions should be stable and usable in browsers before we release Vue 3. Otherwise, even if we keep unstable class features out of the docs, there will still be unstable features in community library examples, blog posts, Twitter, etc - written by people using Babel. As proven in the React community, this is inevitable when an unstable feature is more convenient or enables a useful pattern. And in this case, using a field declaration with:

static props = {
  msg: String
}

is just more convenient than:

static get props() {
  return {
    msg: String
  }
}

Even I'd probably be defining my props using the more convenient field declarations, because they're simpler and also more semantically correct, since a getter is only necessary as a hack to get around the more appropriate feature being unavailable sometimes.

And so then I fear we'll have:

  • Frustrated non-transpiling users, because many of the public code examples they see simply don't work for them. They'll try static props = { and get Uncaught SyntaxError: Unexpected token =. As a result, they might fall back to the object syntax not simply out of familiarity, but perhaps necessity, because at least those examples will work consistently without them having to keep track of which class features are stable.
  • Confused Babel users, because we're not using the more appropriate and convenient (but not quite stable) class features in our docs, in order to avoid confusion for non-transpiling users.

However, if everyone can just use the correct and convenient class features, migrating to class-based components will be much less painful for non-transpilers and we simulataneously avoid confusion for Babel users.

we are not going to deprecate object-based syntax in 3.x, those who prefer it can keep using it.

But as I said, this would be a step towards deprecating the object-based API, right? If we moved to a default class-based API in Vue 3, because we want to get everyone using the same syntax in both JS and TS, I'd be very surprised if we kept the object-based syntax for Vue 4 - so their fears would be warranted.

this is the less "feeling threatened" way to look at it: Components have always been internally represented as classes, even in 2.x (as function constructors). We are just converting options objects into classes. However, in 2.x there's no way to directly author this internal class using the class syntax. 3.0 provides a way to do that, while still keeping the option to use objects.

I don't feel like that's entirely accurate. 😕 If we're moving to classes as the new recommended default for all users, we're doing a lot more than "providing a way" to directly author internal classes - we're creating a significant cost to not using classes, since that's what most public examples will use. I think there are ways to sell classes as an improvement, but it'll be much, much harder if non-transpiling users are stuck using only a subset of the essential DX features for classes and due to instability, often hitting roadblocks as they struggle to learn them.

Note that while there are users who strongly prefer the object syntax, there are other users who strongly prefer the class-based syntax too (especially for users coming from languages where class is the norm).

TypeScript users aside, it sounds like these JS users would feel more "comfortable" or "safe" using classes. 😉 (But as a sidenote, I came from languages where classes are central, which is part of the reason I've been so frustrated with the instability and generally unfinished state ES classes have been in, resulting in poor DX.)

Shying away from classes essentially sacrifices the DX for the class-preferring group where the only benefit is making the object-preferring group feel "comfortable" or "safe".

I'm not shying away from classes at this point. Personally, I still see many more disadvantages than advantages for JS users, both as an educator and developer, but I'm trying really hard to have faith and figure out how to make the transition as smooth as possible.

@Jinjiang
Copy link
Member

How about removing data() and move the job into constructor function?

now:

class App extends Component {
  // data
  data() {
    return { count: 0 }
  }
}

new:

class App extends Component {
  constructor() {
    // data
    this.count = 0;
  }
}

Does it do the same thing with ES6 Proxy now?

Thanks.

@HerringtonDarkholme
Copy link
Member

I really love @Jinjiang 's proposal and actually I have also considered it.

Pros are evident: it removes data method from API so it does simplify API concept. It also removes one generic type Data from TS's definition. It also promotes a good practice that constructor should have only data initialization in it.

Current implementation only assumes component being newable. See https://github.com/vuejs/vue-next/blob/master/packages/core/src/componentUtils.ts#L24

One straight forward way to collect reactive data is to Object.getOwnPropertyDescriptors. Every propery defined/setted in constructor is counted as reactive.

However, users cannot use props in constructor to initialize data, if component class does not extend Vue's Component. This is because we have no way to inject props to an object before its constructor returns.

@phanan
Copy link
Member

phanan commented Oct 11, 2018

It also promotes a good practice that constructor should have only data initialization in it.

While I personally like the idea, I'm not sure about this practice. Constructors can, and often, do more than just initializing the data. Also, not every piece of data initialized in the constructor needs to be reactive, so implicitly treating them that way instead of using an explicit data object is something I'd frown upon. Not to mention, again, backward-compatibilities.

EDIT: While we're at it, constructors are more like created() hooks to me.

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Oct 11, 2018

Constructors can, and often, do more than just initializing the data.

Constructors in general can do a lot of things. But component class isn't general purpose, it is for binding view with data. For component constructors, most of them should do few things (or be dumb). React advises users only initialize state or bind methods. https://reactjs.org/docs/react-component.html#constructor Angular also instruct users to use constructor as state initialization and dependency injection. None of them are implementing business logic. Component class also has lifecycle method like created to implement logic.

not every piece of data initialized in the constructor needs to be reactive,

We can treat the new constructor as the old data function. The object we returned from data has all properties being reactive. And if users don't want certain property to be reactive, prefixing with _ and moving it to created are escaping hatches. They are also applicable to class API.

That said, I'm still ambivalent toward the proposal of treating constructor as data. We will have to rely on hacky or dirty implementation to keep backward-compatible and feature complete(like using prop without inheriting).

@Jinjiang
Copy link
Member

Jinjiang commented Oct 11, 2018

And as a step forward, we can simplify the lifecycle design by deprecating beforeCreated right? Because it’s more like a constructor in v2 I think. 🤔

EDIT:

EDIT: While we're at it, constructors are more like created() hooks to me.

@phanan sorry for missing your comment. It seems we think the same. 😅

@yyx990803
Copy link
Member Author

@chrisvfritz ok, I may have misunderstood your intention. From what you have explained, it seems your concern has less to do with the actual implementation, but more about how we present / document / transition the users to the new syntax. So let's clarify a few points:

  • Given that static class fields will likely land in ES as they are already stage 3, we will eventually be able to get a class API that is decent enough;

  • Vue should eventually have a native class-based API for the following reasons:

    • classes will eventually become a mainstream concept that any ES user will be familiar with;
    • better alignment with internal implementation
    • better alignment with type systems

    And we should start experimenting with that now so that we can have it ready when static class fields become stage 4 and start to be implemented natively in browsers.

  • The timing for 3.0 may not align perfectly well with the landing of static class fields, and your concern seems to be that it would be bad to start promoting classes as the default if that doesn't work out. I agree with that, but I actually don't think we have to push classes as the default in 3.0. The internal implementation has to be there regardless, it's up to us how to present it:

    • If the timing works out (static class fields are widely available in browsers when 3.0 launches), that's great and we can promote classes as the default.

    • If the timing doesn't work out, we shouldn't let it block us from shipping 3.0 because it's not only about classes. Instead, we still recommend objects as the default, and tell users that "you can optionally use classes if you prefer it". This essentially doesn't change anything for existing users while removing some pain points for users who already want to use classes.

Does that make sense?

@yyx990803
Copy link
Member Author

yyx990803 commented Oct 11, 2018

@Jinjiang @HerringtonDarkholme @phanan I've also considered this. Some reasons I am hesitant about it:

  • It requires us to do an Object.getOwnPropertyDescriptors on every instance initialization to dynamically construct the reactive data object. This has some performance cost that I will need to measure.

  • You always need to also call super(). This makes usage without field initializers a bit verbose.

  • Having data() separately allows using initializers for non-reactive properties This can be worked around by just placing non-reactive properties in created()

The biggest advantage is with field initializers + decorators the usage is clean, expressive AND type-checking friendly.

The biggest problem is usage is only nice with field initializers + decorators. Plain ES usage is much more verbose.

@yyx990803
Copy link
Member Author

yyx990803 commented Oct 11, 2018

Ok I've managed to make both data AND initializers work:

// plain ES2015, use `data()` for more concise code
class Foo extends Component {
  data() {
    return {
      count: 1
    }
  }
}

// TS or with Babel, use initializers for the nice syntax
class Foo extends Component {
  count: number = 1
}

With benchmarking the performance cost is in acceptable range (~10ms for 3000 component instances)

One caveat: if you want to access props in initializers, you can, but you must do so via this.$props.someProp, e.g.

interface Props {
  value: number
}
class Foo extends Component<Props> {
  count: number = this.$props.value + 1
}

@chrisvfritz
Copy link
Contributor

chrisvfritz commented Oct 11, 2018

Class education/transition

Does that make sense?

@yyx990803 That makes perfect sense, clarifies quite a few things for me, and generally resolves my education/transition concerns. 🙂 And if we don't end up making classes the recommended default for JS users, all my concerns actually disappear. I'll keep thinking about what might be an ideal learning path and then create a separate issue to discuss that.

Alternatives for data

@HerringtonDarkholme @phanan @Jinjiang @yyx990803 With all of these alternatives (constructor, field initializers), access to props is still an issue, right? Isn't that a dealbreaker? We also lose quite a bit of control over when data evaluates in the lifecycle, which could possibly cause more issues later on.

I can see perhaps working around access to props with special behavior for data with function definitions, e.g.:

class Foo extends Component {
  static props = ['defaultMessage']

  message = props => props.defaultMessage
}

But then do we lose the type-checking benefits, since message would be immediately transformed from a function to another type?

@nonReactive/markNonReactive

I like these much better than Object.freeze, but I'm wondering now, do we actually need a special API for non-reactive properties? When you don't want a property to be reactive, couldn't you just avoid initializing it in data and do so in created/mounted instead?

@phanan
Copy link
Member

phanan commented Oct 11, 2018

do we actually need a special API for non-reactive properties? When you don't want a property to be reactive, couldn't you just avoid initializing it in data and do so in created/mounted instead?

That's a very good point. IMHO reactivity should be opted-in instead of opted-out.

@yyx990803
Copy link
Member Author

yyx990803 commented Oct 11, 2018

@chrisvfritz

do we actually need a special API for non-reactive properties? When you don't want a property to be reactive, couldn't you just avoid initializing it in data and do so in created/mounted instead?

You don't really need them (especially in plain ES), but:

  • The decorator allows TS users to place typing and value initialization of a non-reactive property in the same place. Otherwise, they'd have to declare the type and then assign it in created() separately.

  • markNonReactive is the replacement for Object.freeze() where you may want to skip observation for a sub-tree in your nested data object (for performance reasons).

@DanielRosenwasser
Copy link
Contributor

DanielRosenwasser commented Oct 16, 2018

Sorry, just to clarify for myself, is the idea that plain property declarations with initializers would automatically be marked as reactive? I think that would be way better from a UX POV, as well as a type system POV. I guess the downside is that this really could only work in a Proxy-based reactivity model; you couldn't swap another reactivity model in if you wanted to support older targets, but I don't know how much weight that holds.

@yyx990803
Copy link
Member Author

@DanielRosenwasser yeah, plain initializers would become reactive. I think it's possible to work in older model too, as we are already doing that in vue-class-component.

@yyx990803
Copy link
Member Author

Update: don't know since when but latest Chrome Canary now enables class fields by default, so we can already play with the following API without a transpiler:

class Foo extends Component {
  static props = {
    msg: String
  }

  count = 0

  render() {
    return h('div', [this.msg, this.count])
  }
}

@Jinjiang
Copy link
Member

Update: don't know since when but latest Chrome Canary now enables class fields by default, so we can already play with the following API without a transpiler:

So the Data interface in TS could be removed if it already works, right?

import { h, Component, ComponentWatchOptions } from '@vue/renderer-dom'

interface Props {
  msg: string
}

class App extends Component<Props> {
  static props = {
    msg: String
  }
  
  // data fields
  count: number = 0

  // ComponentWatchOptions type is only needed if `this` inference
  // is needed inside options, e.g. in watch callbacks
  static watch: ComponentWatchOptions<App> {
    count(value) {
      console.log(value)
    }
  }

  created() {
    console.log(this.count)
  }

  get plusOne() {
    return this.count + 1
  }

  increment() {
    this.count++
  }

  render(props) {
    // ...
  }
}

@yyx990803
Copy link
Member Author

@Jinjiang it can still be useful when you somehow have to use data() or need to access the properties on this.$data.xxx, rare cases, but may still be needed.

@chrisvfritz
Copy link
Contributor

or need to access the properties on this.$data.xxx

Oh, so this.$data won't be available unless using a data function?

@znck
Copy link
Member

znck commented Nov 30, 2018

@chrisvfritz I think, Evan meant Component<Props, Data> instead of Component<Props>.

@octref
Copy link
Member

octref commented Feb 20, 2019

avoid defining types twice for each prop

Don't think this concern by @chrisvfritz is addressed. I doubt TS users would use the msg: String runtime type-checking over dev-time type-checking. Is props optional and only for the runtime "validation" purpose?

Are both cases below valid usages in 3.0?

interface Props {
  msg: string
}

// A
class App extends Component {
  render(props: Props) {
    // ...
  }
}

// B: For uses without render function
class App extends Component<Props> {

}

Also, would the class API support default, required and validator for props?

@yyx990803
Copy link
Member Author

yyx990803 commented Feb 20, 2019

@octref yes, that one still needs some more work.

First, runtime props option works the same way as in v2:

class App extends Component {
  static props = {
    msg: {
      type: String,
      default: 'foo',
      validator: str => ['foo', 'bar'].includes(str)
    }
  }
}

Second, in the current prototype the behavior changes based on whether static props is declared:

  • When declared: all declared props are exposed directly on this and also on this.$props. Any props being passed in but not explicitly declared are placed in this.$attrs.

  • When not declared: everything passed in is in this.$props.

So technically, props type inference on this should be based on static props instead of the <Props> interface - but I don't know if it's even possible... @HerringtonDarkholme @Kingwl any ideas?

@octref
Copy link
Member

octref commented Feb 20, 2019

I think this API would be cleaner. It's the same in JS/TS usage.

class App extends Component {
  // props
  static msg = 'foo';
  
  // data
  count = 0;
}

TS advanced usage, for more expressiveness:

type T = 'foo' | 'bar';

class App extends Component {
  // no need to write `static msg: string = 'foo'`, as TS infers type of `msg`
  static msg = 'foo';

  // use TS for more complex types
  static msg2: T = 'foo';

  // runtime prop options for JS/TS
  static msg3 = {
    deafult: 'foo',
    validator: (s: string) => ['foo', 'bar'].includes(s)
  };
}

I think this better aligns Vue's API with the class semantic.

I also like that props: ['msg', 'msg2'] usage can be easily translated to:

class App extends Component {
  static msg;
  static msg2;
}

@yyx990803
Copy link
Member Author

@octref static properties are mapped to options, e.g.

class App extends Component {
  static template = `<div>{{ msg }}</div>`
}

I don't think static properties are semantically correct for props.

@octref
Copy link
Member

octref commented Feb 21, 2019

  • When not declared: everything passed in is in this.$props.

@yyx990803 I reread some discussion in #2 and #3. This Comment was helpful.

  1. If the user provided a Props interface but didn't specify the static props options, the props will still be merged onto this in types but not in implementation.

So are both cases below invalid, because msg does not exist on this in runtime? (Do they have to be $props.msg?)

class App extends Component {
  static template = `<div>{{ msg }}</div>`
}
<template>
  <div>{{ msg }}</div>
</template>

<script>
export class App extends Component {}
</script>

Whereas in TS, does the below case mean:

  • this.msg should be available in methods and render JSX
  • msg should be available in template option and SFC template
interface Props { msg: string }
class App extends Component<Props> {
  static template = `<div>{{ msg }}</div>`
}

I don't think static properties are semantically correct for props.

I do think static properties map semantically to props option but not props as bound to this instance. But now looking back I also see why my proposal could be confusing.

@yyx990803
Copy link
Member Author

So are both cases below invalid, because msg does not exist on this in runtime?

Correct. It may be on this.$props, but definitely not on this.

Whereas in TS, does the below case mean...

No. This is a part where the type is currently mismatched with the runtime behavior. Declaring a props interface currently makes the props available on both this and this.$props in types, but at runtime it's only available on this.$props because there's no runtime props declarations (via static props = ...).

The ideal situation is:

  1. static props = { ... } can be used to infer prop types on both this and this.$props. (seems difficult, AFAIK there's no easy way to augment this type in a class based on its static properties, or is there?)

  2. The props interface only affects this.$props, and is used when the user don't want runtime type checks with static props = { ... }. If we can get (1) working, then this can potentially be dropped altogether.

@octref
Copy link
Member

octref commented Feb 21, 2019

AFAIK there's no easy way to augment this type in a class based on its static properties, or is there?)

Purely with types I don't know. I can give it a try together with @DanielRosenwasser.

  1. The props interface only affects this.$props

I'm not sure, for me I think https://github.com/itsFrank/vue-typescript rather than vue-class-component is what most TS users want. todos: TodoListComponent[] is not expressible with the runtime props option.

For example:

interface Props { msg: MsgItem }
class App extends Component<Props> {
  static props = ['msg'];
}

What TS users want is likely in the template, the msg. completion using the MsgItem interface, and errors for accessing msg.foo if foo does not exist on MsgItem.

@yyx990803
Copy link
Member Author

The problem with decorators is that the proposal still faces a lot of uncertainty at this point, so I'd like to avoid relying on it.

I think it makes sense for the interface to augment this if the runtime declaration is also present - otherwise it's possible for the user to end up with code like this:

interface Props { msg: string }
class App extends Component<Props> {
  created() {
    // make use of this.msg, assuming it's a string
  }
}

where it's valid in types but actually it'd be undefined at runtime.

@yyx990803
Copy link
Member Author

Closing in favor of #20

@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants