-
Notifications
You must be signed in to change notification settings - Fork 431
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 style props
definition
#465
Comments
@ktsn
What if someone has in the same file this line?
|
I don't think you can use type annotations when you using the prop function (at least not the normal way)... baz: string = prop({ default: 'default value' }) The signature from the prop function is.. declare function prop<T>(options: PropOptionsWithDefault<T>): WithDefault<T>; so you need to write the above example like this to be valid.. import { WithDefault } from 'vue-class-component';
baz: WithDefault<string> = prop({ default: 'default value' })
// you need to use the WithDefault interface :(
// or shorter like this..
baz = prop<string>({ default: 'default value' }) my thoughts on this two examples...They are both the same, but... baz = prop<string>({ default: 'default value' }) // valid only on typescript of course
baz = prop({ default: 'default value' }) // valid on typescript and javascript Can you consider renaming the Something like this for example... import { Vue } from 'vue-class-component';
class MyComponent extends Vue.component {
} or import { Component } from 'vue-class-component';
class MyComponent extends Component {
} or both :) and you can pass the props or whatever like an argument on the component.. import { Component } from 'vue-class-component';
class MyComponent extends Component(Props, [Mixins], Whatever) {
} |
There is no
Mixin is also a Vue constructor, hence it will looks like: class App extends mixins(Foo, Bar).props(Props) {} Let's discuss As for I'm not good at wording but maybe we should use abstract words for that like |
I understand, you are right, the cost and the confusion will be big
I like very much the idea of abstract words. how about something like this? // Component allways extends Vue
class App extends Vue {
}
// using Props
class App extends Vue.useProps(Props) {
}
// using Mixins
class App extends Vue.useMixins(Mix1, Mix2) {
}
// Chaining
class App extends Vue.useProps(Props).useMixins(Mix1, Mix2) {
} what you think? |
I, for one, am a very happy user of
It makes code really readable / simple, with everything in the class definitions and not imported types. I understand the concerns with the uncertainty of the spec but we'd just have to move to the next major version of I would further add that:
|
Well, the uncertainty is not the only reason to propose this approach as stated in the original post. And decorator cannot properly type the |
I'm assuming you mean the type of A large library like Vuetify only uses I understand the argument but I think that the convenience of the decorator needs to be weighed against the few times And I understand if the role of |
It's not just perfection but we can utilize props type checking feature in TSX and Vetur. Please carefully read the proposal. |
Regarding the naming, I came up with |
I'm very sad that I didn't know this property before :( The quote is from #447 (comment)
Like you can see in the above example, if you have same property name on The results of this component will be.. <div>
PROP VALUE - PROP VALUE
</div> After specifying <div>
PROP VALUE - CLASS PROPERTY aka Data
</div> Now like you can see the So the REALLY REALLY NICE :) For me this was the behavior I was looking for and I hope no one changes this behavior in the future. I hope in the next version we can access props only through |
Just going to add 2 cents for consideration in regards to decorators. I’ve tried to follow all of the threads regarding the subject and completey understand the direction. My team and I have a major application (100k’s lines of code) and many of the components utilize this library - the decorator approach was very elegant. Even if decorators are not the suggested / ideal, it would be nice to have for backwards compatibility even if it means losing the ability to properly type the Love the library, thanks for all the hard work! |
@ktsn import 'reflect-metadata'
import { Vue, Component, Prop } from 'vue-property-decorator'
@Component
export default class MyComponent extends Vue {
@Prop() age!: number
} If you'd like to set type property of each prop value from its type definition, you can use reflect-metadata. isn't this easier to to use with ts ? |
I read the thread, the answers, the motivations of the proposal and I allow myself to react. First of all, I am grateful for the work and efforts of this library. A big thank-you. I recognize the potential benefit of this proposal but find that there are a few points which have been overlooked (or have been left out).
For me, there is a huge implication in the new proposal. The pure and simple stop of the decorators. |
I'll clarify the following points:
Reflect API (and
Can you show a concrete example of the case that the proposed API becomes an issue by using inheritance? You can just say "inheritance is wrong" but it doesn't push the discussion forward. I know the principle to prefer composition to inheritance and also the case of harmful inheritance when abused but I don't think all inheritance is bad. There are cases that inheritance is properly used - For example, both Android and iOS SDK let developers inherit built in UI components to customize them. There is a similar case of it on the web with custom elements and you need to inherit a native element (e.g. Besides, if you say inheritance is bad, why are you ok with the existing Vue Class Component API because you have to inhertit @Component
export default MyComp extends Vue { // inheriting Vue
} What is the difference from the proposed approach?
If you mean mixins by "multiple inheritances", you can already do that by If you just mean you want to annotate export default StepOne extends StepAbstract.props(Props) {
}
export default StepTwo extends StepAbstract.props(Props) {
}
export default StepThree extends StepAbstract.props(Props) {
}
How does this relate to the proposed API?
I know Angular's approach since I read its code when I implement template type checking in Vetur. Yes, it is "technically" possible because Angular develop everything by themselves - compiler, type checker, language server, etc. In addition, in Vue, it would need more tooling to develop because it also supports render function and TSX. You may have to implement typescript plugin and custom webpack loader to intercept TS type checking mechanism. Why don't we just follow the standard TypeScript rule, then properly type the props so that we don't need to develop and maintain the bunch of toolings?
I believe the discussion of defining props inside vs. outside class turned out trivial and just personal preferences because there are both types of API in the existing UI frameworks which are React and Angular and are well accepted.
I don't understand what you mean by "be handled by typescript / Babel". TS and Babel will just implement the spec, if the spec changes it, of course, affects libraries and even the end users' code. Let me tell you an example. The current class Test extends Vue {
// No initializer class property
@Prop foo
} In TypeScript (without This subtle difference affects the following case: class Test extends Vue {
foo = 'foo + ' + this.bar
@Prop bar
baz = 'baz + ' + this.bar
} In TypeScript, this works without any issues. But in Babel, There are also other APIs that the latest decorator spec does not support while the previous one provides. e.g. accessing property descriptors. These kinds of changes may or may not be able to handle in library code, no one knows about it since the spec is not fixed yet. If we can't handle it when the final decorator spec is implemented, the end users have to rewrite their code to get rid of this issue or Vue Class Component may have to stop providing the decorator. As for the existing decorator that Vue Class Component provides, I concluded to provide it continuously because class decorator is relatively stable - the spec has not been changed so much from stage 1 which the decorator just receives class constructor via the first argument. Also, I don't mean we should stop using every decorator. If the usage is well-matched with decorators and the risk is relatively low, I think it's ok to include. |
@ktsn Thanks for taking the time to respond. The 3 clarified points are important, thank you for clarifying. I will try to be clear in my answer 1. Inheritance is not bad but is not flexible (which is why I used the words: "a wrong way" for your solution). 1bis. Why accept the Vue inheritance as a component? 1 and 1bis This limits how I can code my application. Example:
2. Vue offers different $variables ($el, $parent, ...) in a view instance. What if tomorrow we want to type this. $var in the template? Say I want to type this.$refs (
3. Even if vue property decorator 4. I can't imagine the time and effort it took for the Angular team or the Vue team (+ yours) for the library. But I wouldn't want to use a solution that distorts object-oriented and its principles, otherwise I might as well stop using classes and switch to function composition. 5. It s not just personnal preference. This is literally the development experience (UX for code) and how cognition works. The discussions are the same in design. You can use a glass ketchup bottle and struggle to squeeze the ketchup out or you rethink the bottle, reverse the direction and do it in a soft material that allows you to squeeze the bottle to squeeze the ketchup out. At the end you have your ketchup but the effort is not the same. More seriously, one of the main principles of Vue is the SFC (the single file component). It was successful because everything is in the same file. Everything is visible in one place and linear. My class is already a container. Breaking it into fragments (except for reuse as the benefit gets greater) weighs down the mental design of the code. 6. Thank you for the additional information. I just wanted to say that if we choose to use the decorators we have to live with the disadvantages because it is not standard. What I don't understand is why stop the |
I remember that this issue with |
// example working actually in Vue 2 + vue-property-decorator
@Component
export default class StepAbstract extends Vue {
@Prop()
nextStepPath: string;
public GoToNext(): void {
this.$router.push(this.nextStepPath);
}
}
@Component
export default class StepOne extends StepAbstract {
@Prop()
customer: ICustomer;
get fullname(): string {
return customer.firstname + ' ' + customer.lastname;
}
@Component
export default class StepTwo extends StepAbstract {
@Prop()
payment: IPayment;
get preferedPaymentMethod(): string {
// code removed for brievety
}
}
// How to do with your solution for this example? -> class Props {
nextStepPath: string
}
export default class StepAbstract extends Vue.props(Props) {
public GoToNext(): void {
this.$router.push(this.nextStepPath);
}
}
// ---
class Props {
customer: ICustomer
}
export default class StepOne extends StepAbstract.props(Props) {
get fullname(): string {
return customer.firstname + ' ' + customer.lastname;
}
// ---
class Props {
payment: IPayment
}
export default class StepTwo extends StepAbstract.props(Props) {
get preferedPaymentMethod(): string {
// code removed for brievety
}
} // example working actually in Vue 2 + vue-property-decorator
IValidable interface {
IsValid (): boolean;
}
@Component
export default class PaymentForm {
@Ref ()
refIbanElement: IValidable;
}
<template>
<form>
<iban-element-from-third-party-component ref = "refIbanElement" />
<input type = "submit" disabled = "!refIbanElement.IsValid ()" />
</form>
</template>
// How to do with your logic without decorator for this example? (see point below before to answer) -> You can just use data as it's export default class PaymentForm {
refIbanElement: IValidable;
}
<template>
<form>
<iban-element-from-third-party-component ref = "refIbanElement" />
<input type = "submit" disabled = "!refIbanElement.IsValid ()" />
</form>
</template>
It's still everything is in one place as they are all in
I already answered in the previous post. As for the existing decorator that Vue Class Component provides, I concluded to provide it continuously because class decorator is relatively stable - the spec has not been changed so much from stage 1 which the decorator just receives class constructor via the first argument. Also, I don't mean we should stop using every decorator. If the usage is well-matched with decorators and the risk is relatively low, I think it's ok to include. If you want to use decorator, you can just keep class Props {
foo = prop<string>()
} |
Thank for your answer @ktsn |
@ktsn |
It'll come in minor update on v8. |
I was testing out the new props definition in rc.1 and I noticed that the intellisense in my IDE (Webstorm) seems to break when props are injected through a subclass of So for example: import { Options, prop, Vue } from "vue-class-component";
class Props {
public defaultText = prop<string>({ default: "" });
}
export default class SomeComponent extends Vue.with(Props) {
private inputText = "";
public created(): void {
this.inputText = this.defaultText; // Can hover to see "public Props.defaultText: string"
}
} The above works fine, right down to the -- However when I introduce a new class I made: // RVue.ts
import { Vue } from "vue-class-component";
export default class RVue extends Vue {
// Some misc. stuff here.
}
// SomeComponent.vue
import { prop } from "vue-class-component";
import RVue from "@/RVue";
class Props {
public defaultText = prop<string>({ default: "" });
}
export default class SomeComponent extends RVue.with(Props) {
private inputText = "";
public created(): void {
this.inputText = this.defaultText; // Nothing on hover.
}
} I no longer get intellisense on props, and it shows fields in the Is this an issue with this library or potentially an issue on Webstorm's side? VSCode works fine with the second code snippet. Or maybe I'm missing something with how I'm setting up my inheritance? It seems as though the // RVue.ts
import { Vue } from "vue-class-component";
export default class RVue extends Vue {
// Some misc. stuff here.
}
// SomeComponent.vue
export default class SomeComponent extends RVue {
private upcastTest: RVue = this; // Compiles fine.
} Without introducing the new props style the above works fine, but when props are introduced it throws an error. class Props {
public something = prop<string>({});
}
export default class SomeComponent extends RVue.with(Props) {
private upcastTest: RVue = this; // Error: Type 'this' is not assignable to type 'RVue'.
} In addition, any protected or public properties on |
Possibly related to the inheritance issue. The new That said, during my test runs it seems like it might even prevent field resolution for fields declared within the component class itself when using the component with vue-test-utils. I get Providing prop names in an |
I have the same issue as @brandonson while using
Does that work for you? When I add |
Please note that if |
Thanks to @ktsn for your thorough explanations & for continuously improving this. I get the feeling many other commenters feel the same, and that explains why many people are resistant to losing the light, class-friendly
Just wanted to add that I think your example looks better with an inline class expression ( import { Vue, prop } from 'vue-class-component'
export default class MyComp
extends Vue.with(class {
foo?: string
bar!: string
baz = prop<string>({ default: 'default value' })
}) {
// Rest of component
} Edit: For reference, at some point this stopped working, and I moved to |
What if the name of the property is e.g. 'package'? It is a reserved keyword and can not be used in template, only as a property inside the class. It would be nice to have the option 'name', to specify the prop name. |
Hi @ktsn there is still not the |
[ v8 ] how to use ref ? thanks @ktsn |
Since TypeScript 4.3, `target: "esnext"` indicates that `useDefineForClassFields: true` as the new default. See <microsoft/TypeScript#42663> So I'm explicitly adding this field to the tsconfigs to avoid any confusions. Note that `lit-element` projects must use `useDefineForClassFields: false` because of <https://github.com/lit/lit-element/issues/1030> Vue projects must use `useDefineForClassFields: true` so as to support class style `prop` definition in `vue-class-component`: <vuejs/vue-class-component#465> Popular React state management library MobX requires it to be `true`: <https://mobx.js.org/installation.html#use-spec-compliant-transpilation-for-class-properties> Other frameworks seem to have no particular opinion on this. So I turned it on in all templates except for the `lit-element` one.
When the next version will be released? |
All is nice but
and what I see here is very ugly with Vue.extend. @ktsn please return old method to declare props , it some how worked. |
@ktsn sorry, no need I think I do it myself vue + classes are nice |
Summary
To be able to define component props with class properties. You can use
prop
helper to specify detailed prop options:In TypeScript, you can omit
prop
helper when you only need to define its type (runtime validation does not happen in that case):You need to specify
"useDefineForClassFields": true
for TypeScript compiler option to let Vue Class Component aware of the properties without initializer (in the above examplefoo
andbar
):Motivation
One motivation is to properly type
Props
type parameter of a component for props type checking in TSX and Vetur. TSX can validate props type on compile type thanks to TypeScript:Vetur also offers similar prop type validation on
<template>
block. To utilize these features, we need to properly typeProps
type parameter of a component.The other motivation is less verbosity. Vue's basic
props
option requires us to define props with values then infers the prop type from the value. For example, we have to annotate complex type withPropType
utility:This is relatively verbose compared to the existing
@Prop
decorator approach from vue-property-decorator.Ideally, the new approach should as short as the decorator approach.
Details
We will introduce two API:
Vue.with
static method andprop
helper function.Vue.with(...)
method receives a class constructor that describes the component props. It collects all class properties and generatesprops
option for the component under the hood. It also respects the property types for the props types:Note that we have to specify
useDefineForClassFields: true
component option in TypeScript to make the above code works.We can also specify detailed prop options by using
prop
helper (e.g.default
,validator
). Theprop
helper receives exact same as Vue core'sprops
option object:Note that we have to specify the type of prop via
prop
helper type parameter when we usedefault
value. This is to differentiaterequired
prop and with-default
prop on the type level. That is,required
should be always of typestring
butwithDefault
should be of typestring
in the component while being of typestring | undefined
when it is used on a parent component since it does not have to receive a value. If the type is able to be inferred from the default value, you don't have to specify it.Alternative approaches
Decorator approach
There has been an approach with
@Prop
decorator but there are issues which already described in #447TL;DR
Props
type parameter, then there is no way to check props type.Mixin approach
This is an approach proposed in #447. But it turned out too verbose compared to the decorator approach. There is also a feedback that defining it as a mixin is not intuitive.
The text was updated successfully, but these errors were encountered: