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

[Bug Report][3.0.3] doesn't allow onClick and $children when vueCompilerOptions.strictTemplates=true #16190

Closed
Maxim-Mazurok opened this issue Nov 30, 2022 · 20 comments
Assignees
Labels
typescript wontfix The issue is expected and will not be fixed

Comments

@Maxim-Mazurok
Copy link
Contributor

Maxim-Mazurok commented Nov 30, 2022

Environment

Vuetify Version: 3.0.3
Vue Version: 3.2.45
Browsers: Chrome 107.0.0.0
OS: Linux

Steps to reproduce

  1. clone https://github.com/Maxim-Mazurok/vuetify-tsc-error-reproduction
  2. npm ci
  3. vue-tsc --noEmit -p tsconfig.json --composite false - works
  4. tsc --noEmit -p tsconfig.json --composite false - doesn't work

And I'm getting red squiggly lines in VS Code because of that

Expected Behavior

Works when vueCompilerOptions.strictTemplates in tsconfig.json is true.

Actual Behavior

Only works when vueCompilerOptions.strictTemplates in tsconfig.json is false (default).
image

Reproduction Link

https://github.com/Maxim-Mazurok/vuetify-tsc-error-reproduction

Other comments

Let me know if this should be reported to vue-tsc instead (Volar)

@Maxim-Mazurok
Copy link
Contributor Author

I figured out that the same error can be produced by vue-tsc if I add this into my tsconfig.json:

{
  "vueCompilerOptions": {
    "strictTemplates": true
  }
}

And as you can see here:
https://github.com/johnsoncodehk/volar/blob/574f9778d6f87866f9a1f1234bb3b4e7ec53d252/vue-language-tools/vue-language-core/src/utils/localTypes.ts#L111

It will add Record<string, unknown> to the ComponentProps type which is essentially allowing any props, and this isn't what I want.

I want onClick to be of type MouseEvent as per https://github.com/vuejs/core/blob/fe77e2bddaa5930ad37a43fe8e6254ddb0f9c2d7/packages/runtime-dom/types/jsx.d.ts#L1233

This, however, doesn't really explain the problem with $children

@Maxim-Mazurok Maxim-Mazurok changed the title [Bug Report][3.0.3] tsc doesn't work for tsx files, but vue-tsc does [Bug Report][3.0.3] doesn't allow onClick and $children when vueCompilerOptions.strictTemplates=true Dec 1, 2022
@Maxim-Mazurok
Copy link
Contributor Author

Actually ComponentProps will probably only apply to .vue (html) approach.
For tsx I think this will be used: https://github.com/johnsoncodehk/volar/blob/89b82f92fdc30674b03941c5f17c60df8c46211c/vue-language-tools/vue-language-core/src/languageModule.ts#L85-L91

Which will perform the following on-the-fly patch to the https://github.com/vuejs/core/blob/fe77e2bddaa5930ad37a43fe8e6254ddb0f9c2d7/packages/runtime-dom/types/jsx.d.ts during compilation:
image

@men232
Copy link
Contributor

men232 commented Apr 30, 2023

Partly hotfix for me

// env.d.ts

declare module 'vue' {
	interface ComponentCustomProps {
		onClick?: (e: MouseEvent) => void;
	}
}

@ml1nk
Copy link

ml1nk commented May 2, 2023

Problem still persists in vuetify 3.2.1 with volar 1.6.3.

Is it related or rather will be solved by #17082?

@Maxim-Mazurok
Copy link
Contributor Author

Yeah, it seems to be a potential fix at a glance.
In our project we are re-defining Vuetify components as a mix of the raw Vuetify component + HTML element props. For example, WDialog = defineComponentWithTag(VDialog, "div"). defineComponentWithTag does a bunch of TS voodoo, which also required us to patch vuetify to export a few internal interfaces, etc. But type-wise been working quite well for us so far.

@ml1nk
Copy link

ml1nk commented May 19, 2023

No idea why the types are changed, but nether "AllowedComponentProps" nor "ComponentCustomProps" are working for vuetify components in 3.3.0.

It seems

vue.VNodeProps & vue.AllowedComponentProps & vue.ComponentCustomProps

is missing in every component.

Vuetify 3.2.5 - VRow
image

Vuetify 3.3.0 - VRow
image
image

@KaelWD
Copy link
Member

KaelWD commented May 19, 2023

Vue 3.3 added this: vuejs/core@4c9bfd2#diff-ea4d1ddabb7e22e17e80ada458eef70679af4005df2a1a6b73418fec897603ceR189

It's still there just unwrapped, all those onVnode... methods are from VNodeProps.

@ml1nk
Copy link

ml1nk commented May 19, 2023

It is unwrapped in the resulting d.mts when building on your side, so it does ignore the additions in my project through type augmentation. As such the workaround isn't going to work anymore.

Do you know an easy way to argument the props of a vuetify components directly?

@KaelWD
Copy link
Member

KaelWD commented May 19, 2023

I think vue would have to remove Prettify for that to be possible, unless they add & PublicProps themselves instead of just using it as a default.

@Maxim-Mazurok
Copy link
Contributor Author

Maxim-Mazurok commented May 19, 2023

@ml1nk here's my solution, hope it helps:

vuetify.tsx:

import { defineComponent, type DefineComponent, type IntrinsicElementAttributes } from "vue";
import * as VuetifyComponents from "vuetify/components";

/**
 * To use new Vuetify component (VAnotherComponent, for example):
 *  1. Add its name to `type VuetifyComponentsToDefine`, like so: `type VuetifyComponentsToDefine = Extends<keyof typeof VuetifyComponents, "VBtn" | "VCard" | "VAnotherComponent">;`
 *  2. Check default tag prop of that component in Vuetify documentation link looks like this: https://next.vuetifyjs.com/en/api/v-another-component/#props-tag
 *  3. Declare the new component like so: `export const WAnotherComponent = defineComponentWithTag("VAnotherComponent", "div");`
 */

/**
 * If you wish to use a Vuetify component not on the list - add it here.
 * You might get errors saying something like `Exported variable 'WBtn' has or is using name 'GroupProvide' from external module "bla/node_modules/vuetify/lib/components/index" but cannot be named.`,
 * in that case you need to edit `node_modules/vuetify/lib/components/index.d.ts` and export the interface from the error message, `GroupProvide` in this case. Then run `npx patch-package vuetify` to update the patch. Ensure that you keep comments on top of the patch file, check git diff.
 * This is an attempt for optimization to speed up declarations generation, not sure if it actually helps tho.
 * Also this lets us avoid some repetition, we can import all Vuetify components in one go and don't get a ton of errors mentioned above.
 */
type VuetifyComponentsToDefine = Extends<
  keyof typeof VuetifyComponents,
  // Any new 'VComponent' union types should appear in alphabetical order
  | "VAlert"
  | "VApp"
  | "VBtn"
  | "VCard"
  | "VCardText"
  | "VCardTitle"
  | "VContainer"
  | "VForm"
  | "VListItemSubtitle"
  | "VProgressLinear"
  | "VRow"
  | "VSpacer"
>;
type Extends<T, U extends T> = U;
// prettier-ignore // to keep the last code line that is explained intact
/**
 * Used to convert Vuetify components into components with better type safety.
 * It creates a wrapper over the original VBtn component that comes from Vuetify.
 * The goal is to make it behave exactly the same during the runtime as the original one, while providing better type safety during development.
 *
 * This helps us in a following ways:
 *   - We don't have to rely on `vue-tsc` patching jsx.d.ts on the fly and allowing unknown props, see https://github.com/vuetifyjs/vuetify/issues/16190#issuecomment-1333087978
 *   - We get better type safety by disabling that patching and banning unknown props
 *   - We can use `vueCompilerOptions.strictTemplates: true`
 *   - We can catch common mistakes, such as passing result of function call instead of a function itself: `onClick={alert("clicked WWBtn")}`
 *   - We can catch common mistakes, such as using wrong props `option="flat"` vs `variant="flat"`
 *   - Maintains all the features of the default approach (type safety, autocomplete in IDE and runtime features)
 *
 * NOTE: To better understand types and/or debug:
 *   - set `noErrorTruncation: true` in `compilerOptions` section of tsconfig.json
 *   - use [kimuson.ts-type-expand](https://marketplace.visualstudio.com/items?itemName=kimuson.ts-type-expand) VS Code extension
 *
 * This used to have some serious performance issues, probably because it needed to generate too many combinations for type declarations for every native element, not sure, but optimized versions seems to work fine.
 */
const defineComponentWithTag = <T extends keyof IntrinsicElementAttributes, K extends VuetifyComponentsToDefine>(
  vuetifyComponentName: K, // "VBtn", "VCard", etc.
  tagName: T // "div", "button", etc. Learn more about Generics: https://www.typescriptlang.org/docs/handbook/2/generics.html
) => {
  const VuetifyComponent = VuetifyComponents[vuetifyComponentName];
  // TODO: WI00574114 - Remove this suppressions during development workflow
  // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
  return defineComponent({
    name: vuetifyComponentName.replace(/^V/, "W"), // rename VBtn to WBtn, etc.
    render() {
      if (
        vuetifyComponentName === "VBtn" &&
        (Object.prototype.hasOwnProperty.call(this.$attrs, "href") ||
          Object.prototype.hasOwnProperty.call(this.$attrs, "to"))
      ) {
        // TODO: WI00574114 - Remove this suppressions during development workflow
        // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
        delete (this.$attrs as { href?: string }).href;
        // TODO: WI00574114 - Remove this suppressions during development workflow
        // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
        delete (this.$attrs as { to?: string }).to;
        console.warn(
          "VBtn can't have `href` or `to` props because it'll convert it from <button> to <a>, removing them"
        ); // we need this because in Vuetify sources there is `const Tag = (link.isLink.value) ? 'a' : props.tag` and `const isLink = computed(() => !!(props.href || props.to))`
      }
      return (
        <VuetifyComponent
          {...this.$attrs} // passthrough (hopefully) all the props/listeners/etc., see https://vuejs.org/guide/components/attrs.html
          v-slots={this.$slots} // passthrough (hopefully) all slots (named/scoped/default), see https://github.com/vuejs/babel-plugin-jsx#slot
          tag={tagName} // this is important because we're forcing TS into thinking that, for example, VBtn will also accept props from native <button>, see https://next.vuetifyjs.com/en/api/v-btn/#props-tag
        />
      );
    },
    inheritAttrs: false, // without this one, `tag=button` prop override doesn't seem to work, not entirely sure why or what are implications, check out https://vuejs.org/guide/components/attrs.html#disabling-attribute-inheritance
    /*
           ╔═[ we're saying "forget all the typings this component had or inferred, we will specify/override them manually/forcefully" ]

           ║                  ╔═[ this will be a huge interface from the original VBtn ]
           ║                  ║
           ║                  ║            ╔═[ this combines/extends interfaces, so `{bla: number} & {qwe: string}` will be `{bla: number; qwe: string}` ]
           ║                  ║            ║
           ║                  ║            ║                      ╔═[ creating a Vue component interface, using native HTML element attributes as props, essentially creating type of the native <button> ]
           ║                  ║            ║                      ║
    ╔══════╩══════╗╔══════════╩══════════╗╔╩╗╔════════════════════╩═══════════════════════╗ */
  }) as unknown as typeof VuetifyComponent & DefineComponent<IntrinsicElementAttributes[T]>;
};

// Any new `WComponent` exports should follow alphabetical order
export const WAlert = defineComponentWithTag("VAlert", "div");
export const WApp = defineComponentWithTag("VApp", "div"); // VApp doesn't take a tag prop but TS complains that VApp can't take in children
export const WBtn = defineComponentWithTag("VBtn", "button");
export const WCard = defineComponentWithTag("VCard", "div");
export const WCardText = defineComponentWithTag("VCardText", "div");
export const WCardTitle = defineComponentWithTag("VCardTitle", "div");
export const WContainer = defineComponentWithTag("VContainer", "div");
export const WForm = defineComponentWithTag("VForm", "form"); // VForm doesn't take a tag prop but TS complains that VForm can't take in children without calling this
export const WListItemSubtitle = defineComponentWithTag("VListItemSubtitle", "div");
export const WProgressLinear = defineComponentWithTag("VProgressLinear", "div");
export const WRow = defineComponentWithTag("VRow", "div");
export const WSpacer = defineComponentWithTag("VSpacer", "div");

// Components do not accept `tag` prop
export const WCardActions = VuetifyComponents.VCardActions;
export const WDivider = VuetifyComponents.VDivider;
export const WMenu = VuetifyComponents.VMenu;
export const WSelect = VuetifyComponents.VSelect;
export const WSnackbar = VuetifyComponents.VSnackbar;
export const WTextField = VuetifyComponents.VTextField;

// // prettier-ignore
// /**
//  * Create WBtn component, which is kinda wrapper over the original VBtn component that comes from Vuetify.
//  * The goal is to make it behave exactly the same during the runtime as the original one, while providing better type safety during development.
//  *
//  * This helps us in a following ways:
//  *   - We don't have to rely on `vue-tsc` patching jsx.d.ts on the fly and allowing unknown props, see https://github.com/vuetifyjs/vuetify/issues/16190#issuecomment-1333087978
//  *   - We get better type safety by disabling that patching and banning unknown props
//  *   - We can use `vueCompilerOptions.strictTemplates: true`
//  *   - We can catch common mistakes, such as passing result of function call instead of a function itself: `onClick={alert("clicked WWBtn")}`
//  *   - We can catch common mistakes, such as using wrong props `option="flat"` vs `variant="flat"`
//  *   - Maintains all the features of the default approach (type safety, autocomplete in IDE and runtime features)
//  *
//  * NOTE: To better understand types and/or debug:
//  *   - set `noErrorTruncation: true` in `compilerOptions` section of tsconfig.json
//  *   - use [kimuson.ts-type-expand](https://marketplace.visualstudio.com/items?itemName=kimuson.ts-type-expand) VS Code extension
//  */
// const VBtnDefaultTag = "button"; // from https://next.vuetifyjs.com/en/api/v-btn/#props-tag
// export const WBtn = defineComponent({
//   name: "WBtn",
//   render() {
//     return (
//       <VBtn
//         {...this.$attrs} // passthrough (hopefully) all the props/listeners/etc., see https://vuejs.org/guide/components/attrs.html
//         v-slots={this.$slots} // passthrough (hopefully) all slots (named/scoped/default), see https://github.com/vuejs/babel-plugin-jsx#slot
//         tag={VBtnDefaultTag} // this is important because we're forcing TS into thinking that VBtn will also accept props from native <button>, see https://next.vuetifyjs.com/en/api/v-btn/#props-tag
//       />
//     );
//   },
//   inheritAttrs: false, // without this one, `tag=button` prop override doesn't seem to work, not entirely sure why or what are implications, check out https://vuejs.org/guide/components/attrs.html#disabling-attribute-inheritance
//   /*
//    ╔═[ we're saying "forget all the typings this component had or inferred, we will specify/override them manually/forcefully" ]
//    ║
//    ║                ╔═[ this will be a huge interface from the original VBtn ]
//    ║                ║
//    ║                ║               ╔═[ this combines/extends interfaces, so `{bla: number} & {qwe: string}` will be `{bla: number; qwe: string}` ]
//    ║                ║               ║
//    ║                ║               ║                        ╔═[ creating a Vue component interface, using `ButtonHTMLAttributes` as props, essentially creating type of the native <button> ]
//    ║                ║               ║                        ║
//   ╔╩═╗╔═════════════╩═════════════╗╔╩╗╔══════════════════════╩════════════════════════╗ */
// }) as typeof VBtn & DefineComponent<IntrinsicElementAttributes[typeof VBtnDefaultTag]>;

// const VCardDefaultTag = "div"; // from https://next.vuetifyjs.com/en/api/v-card/#props-tag
// export const WCard = defineComponent({
//   name: "WCard",
//   render() {
//     return <VCard {...this.$attrs} v-slots={this.$slots} tag={VCardDefaultTag} />;
//   },
//   inheritAttrs: false,
// }) as typeof VCard & DefineComponent<IntrinsicElementAttributes[typeof VCardDefaultTag]>;

patches/vuetify+3.1.2.patch (apply using npx patch-package):

Have to export these interface because https://github.com/microsoft/TypeScript/issues/5711

diff --git a/node_modules/vuetify/lib/components/index.d.ts b/node_modules/vuetify/lib/components/index.d.ts
index bf0615f..fe960d0 100644
--- a/node_modules/vuetify/lib/components/index.d.ts
+++ b/node_modules/vuetify/lib/components/index.d.ts
@@ -33,7 +33,7 @@ interface OnColors {
     'on-error': string;
     'on-info': string;
 }
-interface ThemeInstance {
+export interface ThemeInstance {
     readonly isDisabled: boolean;
     readonly themes: Ref<Record<string, InternalThemeDefinition>>;
     readonly name: Readonly<Ref<string>>;
@@ -957,7 +957,7 @@ declare const VAlertTitle: vue.DefineComponent<{
 }>;
 declare type VAlertTitle = InstanceType<typeof VAlertTitle>;

-interface LoaderSlotProps {
+export interface LoaderSlotProps {
     color: string | undefined;
     isActive: boolean;
 }
@@ -965,7 +965,7 @@ interface LoaderSlotProps {
 declare type ValidationResult = string | boolean;
 declare type ValidationRule = ValidationResult | PromiseLike<ValidationResult> | ((value: any) => ValidationResult) | ((value: any) => PromiseLike<ValidationResult>);

-interface VInputSlot {
+export interface VInputSlot {
     id: ComputedRef<string>;
     messagesId: ComputedRef<string>;
     isDirty: ComputedRef<boolean>;
@@ -1256,14 +1256,14 @@ declare const VInput: {
 });
 declare type VInput = InstanceType<typeof VInput>;

-interface DefaultInputSlot {
+export interface DefaultInputSlot {
     isActive: Ref<boolean>;
     isFocused: Ref<boolean>;
     controlRef: Ref<HTMLElement | undefined>;
     focus: () => void;
     blur: () => void;
 }
-interface VFieldSlot extends DefaultInputSlot {
+export interface VFieldSlot extends DefaultInputSlot {
     props: Record<string, unknown>;
 }
 declare type VFieldSlots = MakeSlots<{
@@ -1530,7 +1530,7 @@ declare const VField: {
 });
 declare type VField = InstanceType<typeof VField>;

-interface ScrollStrategyData {
+export interface ScrollStrategyData {
     root: Ref<HTMLElement | undefined>;
     contentEl: Ref<HTMLElement | undefined>;
     activatorEl: Ref<HTMLElement | undefined>;
@@ -1544,7 +1544,7 @@ declare const scrollStrategies: {
     block: typeof blockScrollStrategy;
     reposition: typeof repositionScrollStrategy;
 };
-interface StrategyProps$1 {
+export interface StrategyProps$1 {
     scrollStrategy: keyof typeof scrollStrategies | ScrollStrategyFn;
     contained: boolean | undefined;
 }
@@ -1552,7 +1552,7 @@ declare function closeScrollStrategy(data: ScrollStrategyData): void;
 declare function blockScrollStrategy(data: ScrollStrategyData, props: StrategyProps$1): void;
 declare function repositionScrollStrategy(data: ScrollStrategyData): void;

-interface LocationStrategyData {
+export interface LocationStrategyData {
     contentEl: Ref<HTMLElement | undefined>;
     activatorEl: Ref<HTMLElement | undefined>;
     isActive: Ref<boolean>;
@@ -1565,7 +1565,7 @@ declare const locationStrategies: {
     static: typeof staticLocationStrategy;
     connected: typeof connectedLocationStrategy;
 };
-interface StrategyProps {
+export interface StrategyProps {
     locationStrategy: keyof typeof locationStrategies | LocationStrategyFn;
     location: Anchor;
     origin: Anchor | 'auto' | 'overlap';
@@ -1580,7 +1580,7 @@ declare function connectedLocationStrategy(data: LocationStrategyData, props: St
     updateLocation: () => void;
 };

-interface InternalItem<T = any> {
+export interface InternalItem<T = any> {
     title: string;
     value: any;
     props: {
@@ -3982,7 +3982,7 @@ interface GroupItem {
     value: Ref<unknown>;
     disabled: Ref<boolean | undefined>;
 }
-interface GroupProvide {
+export interface GroupProvide {
     register: (item: GroupItem, cmp: ComponentInternalInstance) => void;
     unregister: (id: number) => void;
     select: (id: number, value: boolean) => void;
@@ -10238,7 +10238,7 @@ interface FormValidationResult {
     valid: boolean;
     errors: FieldValidationResult[];
 }
-interface SubmitEventPromise extends SubmitEvent, Promise<FormValidationResult> {
+export interface SubmitEventPromise extends SubmitEvent, Promise<FormValidationResult> {
 }

 declare const VForm: vue.DefineComponent<{

I also have the in my tsconfig.app.json:

{
  //...
  "vueCompilerOptions": {
    "strictTemplates": true
  }
}

@ml1nk
Copy link

ml1nk commented May 22, 2023

This helps, thank you.

@carlosyan1807
Copy link

image image

@Maxim-Mazurok
Copy link
Contributor Author

@carlosyan1807 I have a feeling that onClick will not have a correct type in your approach and probably will be any, but I haven't actually tried it.

@carlosyan1807
Copy link

image

@Maxim-Mazurok
Copy link
Contributor Author

In your screenshot above it likely has just inferred the type from toggleDark definition, so you still can pass any invalid argument to onClick and it will not throw type errors, see

image

@carlosyan1807
Copy link

Ah, you're right. It seems like this is just making VS Code not show errors, but the type issue still persists.

@men232
Copy link
Contributor

men232 commented Sep 19, 2023

Seems works, but not tested well

declare module '@vue/runtime-core' {
	interface ComponentCustomProperties {
		$props: {
			onClick?: (e: MouseEvent) => void;
		}
	}
}

@KaelWD KaelWD closed this as completed in b6b5583 Oct 25, 2023
@KaelWD KaelWD modified the milestone: v3.3.x Oct 25, 2023
@moeshin
Copy link

moeshin commented Nov 5, 2023

It seems the problem still exists.

image

@jfrs
Copy link

jfrs commented Nov 9, 2023

I'm also still experiencing this and #17404 with Vuetify 3.4.0 and Vue 3.3.8.

@ml1nk
Copy link

ml1nk commented Nov 13, 2023

"AllowedComponentProps" and "ComponentCustomProps" are working again, but this doesn't resolve the bug but enables us to define properties globally as a workaround. Please reopen.

@KaelWD KaelWD added the wontfix The issue is expected and will not be fixed label Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript wontfix The issue is expected and will not be fixed
Projects
None yet
Development

No branches or pull requests

8 participants