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

Detect unknown/undefined prop usage #1077

Closed
IceBlizz6 opened this issue Mar 19, 2022 · 45 comments
Closed

Detect unknown/undefined prop usage #1077

IceBlizz6 opened this issue Mar 19, 2022 · 45 comments
Labels
enhancement New feature or request

Comments

@IceBlizz6
Copy link

This is a proposal for enhancing vue-tsc

Example:

<MyComponent
    :propThatDoesNotExist="42"
/>

When running npx vue-tsc --noEmit i want it to detect that propThatDoesNotExist is not presently defined in myComponent.
This could produce a warning or error.

Feature like this currently exists in TS files.
When trying to edit fields that is not defined in some typescript class it will output:
Property 'xxx' does not exist on type 'MyClass'.

This would be very helpful for Vue developers when renaming props or performs refactoring or git rebases as looking through the code manually can be tedious.

@IceBlizz6 IceBlizz6 changed the title Detect unknown prop usage Detect unknown/undefined prop usage Mar 19, 2022
@johnsoncodehk
Copy link
Member

This is intentional behavior as this is often used in css selectors.

I'll explore add a option in vueCompilerOptions to change this behavior.

@johnsoncodehk johnsoncodehk added the enhancement New feature or request label Mar 20, 2022
@IceBlizz6
Copy link
Author

@johnsoncodehk thanks
would be great to have.
Having to opt in through some configuration is fine.

@sapphi-red
Copy link
Contributor

I also think it is great to have this feature.
IMO data-* and aria-* can be ignored even if it is not used.

@xiaoxiangmoe
Copy link
Collaborator

https://www.typescriptlang.org/docs/handbook/jsx.html#attribute-type-checking
Note: If an attribute name is not a valid JS identifier (like a data-* attribute), it is not considered to be an error if it is not found in the element attributes type.

But it seems that vue always treat attribute data-foo as dataFoo

@IceBlizz6
Copy link
Author

It may be more appropriate to handle this case by giving a warning rather than an error.
The proposed solution is to add option in vueCompilerOptions so maybe users can select option there for themselves of how severe they want to treat this issue, including the possiblity of disabling the check itself

@eladcandroid
Copy link

eladcandroid commented Apr 25, 2022

@johnsoncodehk Do you have any plan for adding that? Maybe we can check with
https://vuejs.org/api/options-misc.html#inheritattrs

@johnsoncodehk
Copy link
Member

johnsoncodehk commented Jun 2, 2022

In 0.36, custom / unknown attrs types behavior handle responsibility will transfer to developer.

  • Expand custom attrs example
<template>
    <div myprop="a">
</template>
// env.d.ts
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        myprop: string
    }
}

export { }
  • Allow any attrs
// env.d.ts

// for native html elements
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        [key: string]: any
    }
    interface SVGAttributes {
        [key: string]: any
    }
}

// for vue components
declare module '@vue/runtime-core' {
    interface AllowedComponentProps {
        [key: string]: any
    }
}

export { }

ouuan added a commit to ouuan/codle that referenced this issue Jun 3, 2022
@lohchab
Copy link

lohchab commented Jun 3, 2022

@johnsoncodehk Thankyou for mentioning that here. Suddenly today my Visual Studio Code started giving 'property does not exist on type' warnings for all the custom attributes starting with 'data-*' like this <h1 data-icon="check">Test</h1> after adding the code in env.d.ts for allow any attrs it solved the issue.

declare module '@vue/runtime-dom' {
interface HTMLAttributes {
[key: string]: any
}
}

export { }

@lohchab
Copy link

lohchab commented Jun 3, 2022

@johnsoncodehk Since this change regarding unknown properties. I am also getting this error on a V-Calendar Components v-model value before this it was fine. I am newbie and don't have any idea how I can omit this warning.

image

@ildan95
Copy link

ildan95 commented Jun 3, 2022

I also think it is great to have this feature. IMO data-* and aria-* can be ignored even if it is not used.

Hello. How to enable ignoring this case?

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 3, 2022

This change is bonkers! How do volar check where this attribute that I passed on purpose ends up? Does it just "assumes" it is the root element of the template? But even that doesn't work properly. Vue has an attribute fallthrough on purpose and this change makes vue-tsc crash on basically every component where I use attribute fallthrough (which I use A LOT).

This feature should be opt-in and not opt-out.

How can I disable this?

@lohchab
Copy link

lohchab commented Jun 3, 2022

I also think it is great to have this feature. IMO data-* and aria-* can be ignored even if it is not used.

Hello. How to enable ignoring this case?

@ildan95 You can add the below listed code in your env.d.ts file and it will allow you to use the data-* and aria-* properties.

// env.d.ts
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        [key: string]: any
    }
}

export { }

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 3, 2022

@johnsoncodehk your solution does not work for components. I cant even pass an id attribute to a component

@lohchab
Copy link

lohchab commented Jun 3, 2022

@Fuzzyma His solution has worked for me and now I am able to add any properties and Visual Studio Code dont show up any warnings for them just like earlier.

Add this to your env.d.ts file in your Project Root Folder.

// env.d.ts
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        [key: string]: any
    }
}

export { }

@Harkonnen13
Copy link

Harkonnen13 commented Jun 3, 2022

@lohchab I can't make it work on components
image

LIke on native elements it works fine, but what about component attributes? I have VInput component which is just a wrapper around native input. SO i have to describe ALL native attributes in props to apply them?

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 3, 2022

exactly. it might work for html elements but attribute fallthrough for components doesn't work at all

@lohchab
Copy link

lohchab commented Jun 3, 2022

@lohchab I can't make it work on components image

LIke on native components it works fine, but what about component attributes? I have VInput component which is just a wrapper around native input. SO i have to describe ALL native attributes in props to apply them?

I got you. Yes, you are right I can confirm its is not working. Its only working for HTML Elements.

For a temporary workaround till the time there's a no solution to this my recommendation is that you switch your Volar Extension Version to the '0.35.2' and Restart the Visual Studio Code.

image

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 3, 2022

Until this is fixed, the proper solution is to revert this feature or hide it behind a flag and release a new version because in the current state it's just plane broken

@johnsoncodehk
Copy link
Member

@lohchab I can't make it work on components image

LIke on native elements it works fine, but what about component attributes? I have VInput component which is just a wrapper around native input. SO i have to describe ALL native attributes in props to apply them?

You should use AllowedComponentProps interface, please check updated example code.

@lohchab
Copy link

lohchab commented Jun 3, 2022

// env.d.ts

// for native html elements
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        [key: string]: any
    }
}

// for vue components
declare module '@vue/runtime-core' {
    interface AllowedComponentProps {
        [key: string]: any
    }
}

export { }

Thankyou @johnsoncodehk I can confirm adding this code to env.d.ts file has solved the unknown property problems for both HTML and Vue Component Elements. Keep up the good work. We all appreciate what you are doing on Volar!

@keithbrink
Copy link

I agree that this type of strict linting should be opt-in, fallthrough attributes are a common use case with Vue and the default configuration of Volar should just work with the examples from the Vue docs.

@henribru
Copy link

henribru commented Jun 3, 2022

This isn't just about fallthrough either, as shown in #1383 this is causing errors for many completely standard HTML attributes directly on the elements. It seems unreasonable requiring people to add custom declaration files to avoid that

@Harkonnen13
Copy link

Is it possible to make an option like mode="strict" to change default behavior? I mean, props existence check looks like an edge case than something default. Because everything that not described in props are custom attributes, that's vue default behavior

@rffl
Copy link

rffl commented Jun 4, 2022

In 0.36, custom / unknown attrs types behavior handle responsibility will transfer to developer.

  • Expand custom attrs example
<template>
    <div myprop="a">
</template>
// env.d.ts
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        myprop: string
    }
}

export { }
  • Allow any attrs
// env.d.ts

// for native html elements
declare module '@vue/runtime-dom' {
    interface HTMLAttributes {
        [key: string]: any
    }
}

// for vue components
declare module '@vue/runtime-core' {
    interface AllowedComponentProps {
        [key: string]: any
    }
}

export { }

It didn't work unfortunately as the project that I'm working on at the moment is using pure JavaScript without TypeScript, and the errors always appear.

@SegaraRai
Copy link

SegaraRai commented Jun 4, 2022

Thanks to this feature, I noticed that the Vuetify property names had been updated and was able to correct them. :)
However, it took some work to resolve the issue. FYI:

  • I had to explicitly add @vue/runtime-core and @vue/runtime-dom to dependencies (probably because I use pnpm).
  • The .d.ts file must be separate from the file defining the vite's ImportMetaEnv (if any).

For now, I use the following definition in env2.d.ts.

import '@vue/runtime-core';
import '@vue/runtime-dom';

// for vue components
declare module '@vue/runtime-core' {
  export interface AllowedComponentProps {
    [key: string]: any;
  }
}

// for native html elements
declare module '@vue/runtime-dom' {
  export interface HTMLAttributes {
    // allow any data-* attr
    [key: `data${string}`]: string;
  }
}

export {};

Complex libraries such as Vuetify do not always have all properties defined (especially if they are passed through), and users will end up having to disable the feature completely.
The ideal solution would be for the type definitions of Vuetify components to be updated, but this is not always done quickly.
It seems difficult to use without at least a way to define additional attributes for individual components.

@johnsoncodehk
Copy link
Member

johnsoncodehk commented Jun 4, 2022

In v0.36.1 this behavior is control by vueCompilerOptions.experimentalSuppressUnknownJsxPropertyErrors and default enabled, if you want report unknown props you need config:

// tsconfig.json
{
  "vueCompilerOptions": {
    "experimentalSuppressUnknownJsxPropertyErrors": false
  }
}

If you see how volar hacking JSX types to ignore unknown prop errors, you may want to disable it. (This shouldn't be part of volar's control)

https://github.com/johnsoncodehk/volar/blob/104af79387fec40ec3c9e82299f1258f1947673e/packages/vue-typescript/src/typescriptRuntime.ts#L282-L290

@TheDutchCoder
Copy link

TheDutchCoder commented Jun 4, 2022

Can you make this disabled by default instead?

I still don't understand what the real point is, as this change breaks totally valid code all over the place for known attributes like all aria attributes.

I'm sure there's good intentions here, but people (especially new users) will have no idea what is going on when their data- attributes are throwing errors suddenly.

@johnsoncodehk
Copy link
Member

@TheDutchCoder Maybe there are some misunderstood, "SuppressUnknownJsxPropertyErrors" mean ignore unknown prop errors, I think you don't want disabled it by default.

@TheDutchCoder
Copy link

Okay I'll give it a spin later.

Just to clarify: data- and aria- attributes are not unknown jsx attributes/properties.

So regardless of the setting, these should never throw errors, which was the original issue.

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 4, 2022

I can confirm that the update fixes all issues. The feature is now opt-in as described above. Thank you for the quick fix!

@lucas-labs
Copy link
Contributor

As @TheDutchCoder said, data-* and aria-* are not unknown attributes, specially on "plain" html elements. They are standard W3C recommendations. The following piece of code is valid html and it should not throw an error regardless of the setting:

<template>
    <input aria-describedby="emailHelp">
    <p id="emailHelp">aria help...</p> 
</template>

@johnsoncodehk
Copy link
Member

As @TheDutchCoder said, data-* and aria-* are not unknown attributes, specially on "plain" html elements. They are standard W3C recommendations. The following piece of code is valid html and it should not throw an error regardless of the setting:

<template>
    <input aria-describedby="emailHelp">
    <p id="emailHelp">aria help...</p> 
</template>

Fix by ba2835f.

@TheDutchCoder
Copy link

TheDutchCoder commented Jun 5, 2022

@johnsoncodehk
I still can't get this to work properly.

If I disable experimentalSuppressUnknownJsxPropertyErrors I still get errors thrown for data- and aria- attributes.
If I enable it, invalid attributes don't throw errors at all.

Example:
The data-test-id attribute should not throw an error, but the thisisnotvalid attribute should:
image

Edit: maybe this isn't released yet?
I'll downgrade in the meantime again

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 5, 2022

@TheDutchCoder if you don't have the option set, it's on by default. Which means volar behaves as if this feature wasn't enabled at all (its a feature flag). Nothing else was changed afaik.

@TheDutchCoder
Copy link

This still doesn't solve anything.

Setting this to true means that unknown attributes (e.g. foo="bar") will go unchecked.
Setting this to false means that known attributes throw errors (e.g data-foo or aria-disabled).

@IceBlizz6
Copy link
Author

I am unable to get this to work with the latest version.
vue-tsc 0.38.3 will report errors of unknown props like expected.
later versions reports no errors, even when it should.
I have tested 0.38.4, 0.38.5, 0.39.4

Can anyone confirm that this feature still works as expected?
if not then i suspect it broke in 0.38.4

@johnsoncodehk
Copy link
Member

@IceBlizz6 You should use vueCompilerOptions.strictTemplates, see https://github.com/johnsoncodehk/volar/blob/master/CHANGELOG.md#0384-2022711.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests