Skip to content

[discussion] Add LifecycleHook interface definition #338

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

Closed
CarterLi opened this issue May 31, 2019 · 4 comments · Fixed by #371
Closed

[discussion] Add LifecycleHook interface definition #338

CarterLi opened this issue May 31, 2019 · 4 comments · Fixed by #371

Comments

@CarterLi
Copy link

CarterLi commented May 31, 2019

// lifecycle.d.ts
import Vue from 'vue';

interface LifecycleHook {
  /** Called synchronously immediately after the instance has been initialized, before data observation and event/watcher setup. */
  beforeCreate?(): void;
  /** Called synchronously after the instance is created. */
  created?(): void;
  /** Called right before the mounting begins: the render function is about to be called for the first time. */
  beforeMount?(): void;
  /** Called after the instance has been mounted, where el is replaced by the newly created vm.$el. */
  mounted?(): void;
  /** Called when data changes, before the DOM is patched. */
  beforeUpdate?(): void;
  /** Called after a data change causes the virtual DOM to be re-rendered and patched. */
  updated?(): void;
  /** Called when a kept-alive component is activated. */
  activated?(): void;
  /** Called when a kept-alive component is deactivated. */
  deactivated?(): void;
  /** Called right before a Vue instance is destroyed. */
  beforeDestroy?(): void;
  /** Called after a Vue instance has been destroyed. */
  destroyed?(): void;
  /** Called when an error from any descendent component is captured. */
  errorCaptured?(err: Error, vm: Vue, info: string): boolean | void;
}

// Usage
export default class YourComponent extends Vue implements LifecycleHook {
  mounted() {
  }
}

Inspired by Angular lifecycle hook interfaces.

Instead of define every hook method in separated interfaces, I prefer defining them in one interface in order not to introduce redundant implement OnXxxs. But at the same time as all of those methods are optional, it generally disables the strict type checking, which makes it mainly for IDE's intellisense.

@Zaba505
Copy link

Zaba505 commented Jun 24, 2019

@CarterLi I definitely think this should be added; however, I disagree with adding all of them in one interface. The reason Angular has all the OneXxx interfaces is because it follows the principle of separation of concerns. Another reason is, like you said,

it generally disables the strict type checking

So, why would we choose to not use TypeScripts' types? Especially, when that's the main reason people use TypeScript.
Lastly, it leads to more explicit code making skim-reading easier.

@ktsn
Copy link
Member

ktsn commented Nov 3, 2019

I doubt this approach is useful because it is completely optional. Developers probably forget to add such interface and just declare hooks without it.
I would say this should be a part of project coding guideline rather than including this lib.

@CarterLi
Copy link
Author

CarterLi commented Nov 4, 2019

It's a common approach in Vue, for example DirectiveOptions:

export interface DirectiveOptions {
  bind?: DirectiveFunction;
  inserted?: DirectiveFunction;
  update?: DirectiveFunction;
  componentUpdated?: DirectiveFunction;
  unbind?: DirectiveFunction;
}

In my opinion those interfaces are very useful because they enable intellisense of IDEs, which generally increases coding speed and reduces chances of typos.

image

@ktsn
Copy link
Member

ktsn commented Nov 11, 2019

That makes sense. Probably it would be better to augment Vue instance type itself rather than providing interface if that is only for the sake of auto-complete.

@ktsn ktsn closed this as completed in #371 Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants