Page MenuHomePhabricator

Improve the way WVUI ships type definitions
Open, Needs TriagePublic

Description

Currently, WVUI includes type definitions (which are auto-generated from the TS compilation process) in its published NPM package. I think there are several ways that we could improve on what we are currently doing here.

1. Organization: WVUI packages the contents of the dist/ folder for publication to NPM. Inside this folder, the type declarations live inside a subfolder called src/; within this directory, there are separate d.ts files for everything in WVUI's source code. Separate files exist for each component, they get imported through the entries files, and we're shipping declaration files not just for components but for tests, storybook stories, and internal utilities in addition to our component declarations.

This seems somewhat counter-intuitive and confusing to me, and there's also a lot of stuff here that really doesn't need to be included in the first place (declarations for tests and stories, etc).

Proposal: we should place type declarations somewhere more predictable like a dedicated types folder, and we should include a top-level index.d.ts file that includes most declarations inline. We should also stop shipping types for library internals and development tooling that are not going to be useful for consumers.

2. Component Type Declarations: I think our current component type declarations could be improved. Here's what the declaration looks like for Button.vue:

import { ButtonType } from './ButtonType';
import { ButtonAction } from './ButtonAction';
import Vue from 'vue';
declare const _default: import("vue/types/vue").ExtendedVue<Vue, unknown, {
    onClick(event: Event): void;
}, {
    rootClasses: Record<string, boolean>;
}, {
    action: ButtonAction;
    type: ButtonType;
}>;
/**
 * A button wrapping slotted content.
 *
 * @fires {Event} click
 */
export default _default;

This comes through in an editor like VSCode like this:

There's a lot of information here but I'm not sure how helpful this actually is. The series of arguments to ExtendedVue are Vue, data, methods, computed, and props respectively, but this isn't super clear for an end-user. Information about the button's action prop (which can only accept certain string values) doesn't actually show up when you use WvuiButton in a template as a consumer (which is really what we want here I'd argue). Same for the events.

I think we may be better off doing what Vuetify does – shipping a declaration file that just states that every component in the library is of type Vue component. So in that case we'd just want something like this:

import { VueConstructor } from 'vue';
declare const WvuiButton: VueConstructor;
export default WvuiButton;

We could combine this with point 1 and have a single file that exports declarations for every WVUI component. For custom prop types like ButtonAction, we could still expose interfaces that a consuming TS application could import directly if one wanted to benefit from type-checking where the component is being used. See here for an example of how Vuetify exports interfaces for things like DataTableHeader, etc.

Here's an example of how this could work. Even if WVUI is being consumed as a CommonJS module delivered via ResourceLoader, a user could still import the packaged types directly from the NPM module (which would be installed as a devDependency in this case). Then if you need to create a button action to pass as a prop, you could do this:

/**
 * @type {import('wvui').ButtonAction}
 */
const action = "progressive";

This would work in both TS and in plain JS.

Proposal: Replace our automatically-generated declaration files with manually-maintained d.ts files for all public components and other interfaces (ButtonAction, etc) that will be relevant to consumers. Most components should just declare the basic VueConstructor type.

3. Ship Component Data for Vetur: I mentioned above that the types we're currently shipping for component props, etc don't show up in Vue template blocks, which is really where we want to see the documentation and completion features. Fortunately there is a solution here, we just need to rely on the Vetur tooling used in editors like VSCode to display this feedback to users. The Vetur docs provide some information on how to do this. Basically, we'd need to add a vetur key to WVUI's package.json and use that to point to data for tags and attributes.

Writing all of this manually would get pretty tedious, but as with types we may want to study what Vuetify is doing here. In their case, they have written some custom scripts to generate JSON based on component documentation. We can probably adapt this solution for WVUI.

Proposal: Start shipping Vetur component data in the WVUI NPM package. Ideally we can generate this automatically.

Event Timeline

egardner updated the task description. (Show Details)

Point 1: agreed.

Point 2: the ExtendedVue thing doesn't look pretty and isn't helpful in the view that you showed, but it does enable TypeScript to know which props and methods exist and what their types/signatures are. This is helpful because if you do need to call a method on a component (not super common, but it happens), TypeScript will type check your method call. So I would advocate for keeping that.

To illustrate what that looks like in practice: if I have <wvui-options-menu ref="menu" /> in my template, then in JS I can do ( this.$refs.menu as InstanceOf<typeof WvuiOptionsMenu> ).handleKeyboardEvent( event ), and the IDE will show me the method documentation for that method, and TypeScript will type check the parameters and return value. You don't get this if everything is just a generic VueConstructor.

Point 3: sounds good, but could you link to an example of what the output of this would look like, to illustrate?

Point 2: the ExtendedVue thing doesn't look pretty and isn't helpful in the view that you showed, but it does enable TypeScript to know which props and methods exist and what their types/signatures are. This is helpful because if you do need to call a method on a component (not super common, but it happens), TypeScript will type check your method call. So I would advocate for keeping that.

To illustrate what that looks like in practice: if I have <wvui-options-menu ref="menu" /> in my template, then in JS I can do ( this.$refs.menu as InstanceOf<typeof WvuiOptionsMenu> ).handleKeyboardEvent( event ), and the IDE will show me the method documentation for that method, and TypeScript will type check the parameters and return value. You don't get this if everything is just a generic VueConstructor.

Ok, that's good to know. In that case I will concentrate on trying to organize things a little better. For the components themselves, their types get surfaced in the main WVUI entry point file (the type declaration generated from this file is what gets referenced by the types key in package.json), which handles the use case you describe. I'd say that an equally-important use case would concern use of rich property objects or events – things that would have type definitions the consumer may want to rely on in an application. Ideally the user should be able to just import { somePropType } from "@wikimedia/wvui" in a TS file or @type JSDoc comment, without needing to dig through the folder structure inside their node_modules.

Point 3: sounds good, but could you link to an example of what the output of this would look like, to illustrate?

This blog post has a good example of what the editing experience would look like: https://itnext.io/vue-intellisense-in-vscode-33cf8860e092

In code, we'd need three things. A vetur key in package.json that points to JSON files for attributes and tags, and then those files themselves. The content of the files would look like this:

Attributes

{
  "star-rating": {
    "attributes": [ "value", "star-count" ],
    "description": "A StarRating component"
  }
}

Tags

{
  "star-rating/value": {
    "type": "number",
    "description": "The star count the rating represents. Eg. 2 could mean 2/5 stars."
  },
  "star-rating/star-count": {
    "type": "number",
    "description": "The total amount of stars to render. Eg. 5 could mean the rating is \"out of 5\"."
  },
}

Obviously, generating this for each property of every component would be pretty tedious, but I think we can look at the scripts used by libraries like Vuetify to automate this for alternatives: https://github.com/vuetifyjs/vuetify/blob/master/packages/api-generator/src/export.js

Change 699460 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[wvui@master] [types] Move type declarations into "types" dir, exclude unneeded files

https://gerrit.wikimedia.org/r/699460