fix: use .ts/.tsx extension for internal virtual .js/.jsx codegen files

[rchl]

This ensures that the virtual files use ts instead of js and tsx instead of jsx extensions. The main reason for me was to make the experience better when looking at virtual files written using Write Virtual Files command but I've also noticed that it improves behavior in one of the tests.

In #3121 a Non-null assertions can only be used in TypeScript files.ts(8013) was fixed and a test added. While the error was no longer shown, the type of slotFilters was incorrectly inferred as any. With this change, the type is actually correct and the test complains about slotFilters being undefined so added the missing variable.

Resolves #3347

[johnsoncodehk]

We do need to use ".js" / ".jsx" exactly, otherwise no error will be shown when using the type in script block, but vue compiler throws.

https://play.vuejs.org/#eNp9kUFLAzEQhf9KnMsq1BbprWwLKgX1oKKCl1zKdrpNzSYhmdSFZf+7kyxdPUhvmffeDN9kOrh1bnqMCAsoQ+WVIxGQoltJoxpnPYlOeNyJXuy8bUTB0UIaaSprAokm1GKZ/DKQV6ZeXRYPqLUVn9br7UVxJU05G8byQC4IG6c3hFwJUe5vVl2Xh/R9OeMqq8q4SOJ43dgt6qUE9iWwVc7GbpgABUbYqXp6CNYwfZd6JVS2cUqjf3GkGFHCQmQneRsm+37KGvmIk5Ne7bH6+kc/hDZpEl49BvRHlDB6tPE10mCv35+x5fdoMnnUnD5jvmGwOibGIXYXzZax/+Qy7WO+Af/sR1i3hCaclkqgKdnnvAS+y/2Z1X9x59N57pOmh/4HwH6pLA==

[rchl]

Sorry, I'm not getting this example (it could be just me not being familiar enough with the code).

In your example you have a JS script block that uses typescript syntax. I can fully get why that won't work. But my change, to me at least, doesn't seem relevant to that as I'm changing extension of the internal generated files that contain typescript code already.

[johnsoncodehk]

I added a failed test to illustrate this.

[rchl]

I added a failed test to illustrate this.

Still don't quite get your point. This test errors with

#3353/main.vue(2,7): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.

which to me looks like a valid error since the argument to at is expected to be a number.

To me this looks like a change for the better - it catches an issue that wasn't caught before?

[rchl]

And if your point is that Volar doesn't highlight the error in the editor then I can also see that behavior in the currently released v1.8.4.

[rchl]

@johnsoncodehk would you be able to respond to my comments?
To summarize: I still can't see any issues that this causes but I can see that it improves some cases.

[rchl]

This generally improves cases when vue component uses JS instead of TS.

#3350 also made the original issue worse, making this fail in more cases. For example this PR also fixes a case like below where number-prop is not properly validated.

main.vue

<template>
    <div>
        <my-component number-prop="not_a_number" />
    </div>
</template>

<script>
import { defineComponent } from 'vue';
import MyComponent from './my-component.vue';

export default defineComponent({
    components: {
        MyComponent,
    },
});
</script>

my-component.vue

<template>
    <div>hi from my component</div>
</template>

<script>
export default {
    props: {
        numberProp: {
            type: Number,
            required: true,
        },
    },
};
</script>

[johnsoncodehk]

We need to make sure that lang="js" (default lang) is consistent with the behavior of .js, if you create a .js file and write ''.at('0'); you will not see errors unless explicitly enable checkJs in tsconfig.

In addition to causing inconsistencies, I believe these checks are unexpected for many JS users.

[johnsoncodehk]

#3350 also made the original issue worse, making this fail in more cases. For example this PR also fixes a case like below where number-prop is not properly validated.

This case is not because #3350, it's because defineComponent cannot provide $props type in js for my-component.vue.

https://play.vuejs.org/#eNp9UU1PAjEQ/SuTxgRIyHLgtqJGDQc9IFGPjQnuDljcndZ+AIbsf3fahZUYdQ/b9r03nfeme3FtTLYJKHIxcYVVxl9KUrXR1sMeSlwqwlvNZ0Ly0MDS6hp6XNA7lySp0OQ8RAFc/FT395IAjNXG5ZD2ABTqV7RzxjoIwH8azGGWqOERtPgRlMUyB28DHuAmrfHfDFL/PuE2te8PBtlZ6iVpNGqvaL+XK0mT0SGbGArv2PRSrbK108Sxkw0pCr5EVWgfjFccSorOoBSLqtLb+4SdmOGaNyzef8HXbhcxKeYWHdoNStFxfmFX6Ft6+jTDHe87stZlqFj9D/mITlchemxlN4FKtn2iS27v0hMqWj276c4juWOoaPQ4xKjmp4zz+yv6t91xNk51PHvRfAE5f7fS

The behavior of this PR is the same as manually replacing all <script> in the project with <script lang="ts">. Will you consider doing this?

[rchl]

The behavior of this PR is the same as manually replacing all <script> in the project with <script lang="ts">. Will you consider doing this?

What do you mean? Where are you suggesting this should be done? In the Volar's code?

[johnsoncodehk]

I mean you can add lang="ts" to SFC script blocks with global replace in your project, so you end up seeing the same type behavior as the result of this PR.

[rchl]

No, I can't do that. My project will not support TS in script blocks by default. Volar would be fine with that but Webpack won't unless I'll add extra loader.

This suggestion is kinda missing the point though. We are looking into fixing an issue in Volar, not changing the project to workaround the issue.

Also note that I'm seeing different results in Vue 2 and Vue 3 with latest Volar release (without this fix):

Vue 3:

Vue 2:

So the issue you've pointed out with $props only affects Vue 3.

[rchl]

Also, you still haven't pointed out any downsides of this change so I'm not sure why are you hesitant in doing that.

[johnsoncodehk]

I've answered that it causes the script block default type behavior to be inconsistent with .js, it's not a matter of trade-off, consistency is a necessary feature.

If you really need to solve it from Volar, you can consider updating the PR to make the default language user-configurable instead of fixed ts.

[rchl]

I've answered that it causes the script block default type behavior to be inconsistent with .js, it's not a matter of trade-off, consistency is a necessary feature.

Sorry but I don't see any specific arguments for where it creates inconsistency.

This change affects the type and extension of codegen files. Having those have JS type is IMO never correct because those are typescript files with type annotations and other typescript specific code.

When using the "Write Virtual Files" command and opening them in the editor, typescript will highlight many issues (mostly Type annotations can only be used in TypeScript files. ​typescript(8010)) with the files as they are clearly not JS files.

[rchl]

We need to make sure that lang="js" (default lang) is consistent with the behavior of .js, if you create a .js file and write ''.at('0'); you will not see errors unless explicitly enable checkJs in tsconfig.

In addition to causing inconsistencies, I believe these checks are unexpected for many JS users.

Sorry, I've missed this argument before as it was followed by another comment immediately.

I see how this change could surprise some users that don't have checkJs enabled and don't expect to see any errors.

OK then, would it make sense to then only apply this logic when writing generated codegen files to disk? That was my original motivation anyway.

[rchl]

I now agree with you about this change being wrong in a way but at the same time looking at virtual files and seeing a bunch of errors from TS doesn't seem right either:

Not sure if there is a better way to handle this in TS.

For now closing this and extracting the changes to tests into #3353