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

test: add tests for Vue 2 #3351

Merged
merged 3 commits into from
Oct 22, 2023
Merged

test: add tests for Vue 2 #3351

merged 3 commits into from
Oct 22, 2023

Conversation

rchl
Copy link
Collaborator

@rchl rchl commented Jul 2, 2023

In Vue 3.x the DefineComponent types seems to by default merge class and style props from AllowedComponentProps into the resulting component. It was added in vuejs/core#1614.

In Vue 2.7 the DefineComponent doesn't have that logic (https://github.com/vuejs/vue/blob/49b6bd4264c25ea41408f066a1835f38bf6fe9f1/types/v3-define-component.d.ts) so we probably have to fix it on volar side.

Fix by merging JSX.IntrinsicAttributes with component props when strictTemplates is enabled and on Vue 2.7.

There are also other similar places below in __VLS_asFunctionalComponent that check strictTemplates option but I wasn't sure in which cases those are used so haven't touched them. If you could help me figuring out testcases that would trigger those then I could update those too.

BTW. Added vue-test-workspace-vue-2 folder that includes fixtures that use Vue 2.7 as only Vue 3.x was tested before. Mostly copied code from vue-test-workspace and added new unknownProp/main.vue test.

@johnsoncodehk
Copy link
Member

We should probably handle this in vue 2.7 types, I need to confirm this with team.

@xiaoxiangmoe
Copy link
Collaborator

Here is my temp solution:

// file: shims-vue.ts
import type { IfAny } from 'vue/types/common.js';
 

// components.d.ts
declare module 'vue' {
    /**
     * @internal
     * @deprecated ⚠️,do not import this
     */
    export const Transition: new () => { $props: any };

    /**
     * @internal
     * @deprecated
     */
    export type VNodeProps = {
        key?: string | number | symbol;
        ref?: any;
        ref_for?: boolean;
        ref_key?: string;
        onVnodeBeforeMount?: any;
        onVnodeMounted?: any;
        onVnodeBeforeUpdate?: any;
        onVnodeUpdated?: any;
        onVnodeBeforeUnmount?: any;
        onVnodeUnmounted?: any;
    };
    /**
     * @internal
     * @deprecated
     */
    export type AllowedComponentProps = {
        class?: unknown;
        style?: unknown;
    };
}

export type Slot<T extends any = any> = (...args: IfAny<T, any[], [T] | (T extends undefined ? [] : never)>) => any;
type InternalSlots = {
    [name: string]: Slot | undefined;
};
export type Slots = Readonly<InternalSlots>;
declare const SlotSymbol: unique symbol;
export type SlotsType<T extends Record<string, any> = Record<string, any>> = {
    [SlotSymbol]?: T;
};
type StrictUnwrapSlotsType<S extends SlotsType, T = NonNullable<S[typeof SlotSymbol]>> = [keyof S] extends [never]
    ? Slots
    : Readonly<T>;
declare function defineSlots<S extends Record<string, any> = Record<string, any>>(): StrictUnwrapSlotsType<
    SlotsType<S>
>;

type _defineSlots = typeof defineSlots;

declare global {
    const defineSlots: _defineSlots;
}

import { useSlots } from 'vue';
(globalThis as any).defineSlots = useSlots;

We may need to add this to vue 2.7

@rchl
Copy link
Collaborator Author

rchl commented Jul 5, 2023

I'd be very happy to have that fixed in vue types if that's reasonable to expect. I wasn't sure if bigger changes like that would still be considered for Vue 2.7.

@xiaoxiangmoe
Copy link
Collaborator

cc @sodatea

@rchl
Copy link
Collaborator Author

rchl commented Jul 5, 2023

And BTW. Your code made me realize that this should also be handled for other Vue attributes like key and ref.

@johnsoncodehk
Copy link
Member

We'll be updating the types for Vue 2.7, but I think this PR can be kept to add tests.

@rchl
Copy link
Collaborator Author

rchl commented Jul 13, 2023

Reverted the change and left the test but this will obviously keep failing until Vue 2 types are updated.

@rchl
Copy link
Collaborator Author

rchl commented Aug 4, 2023

@xiaoxiangmoe @sodatea Can I track the progress of fixing this in Vue core somewhere? Is there an existing issue for it or should one be created?

@so1ve so1ve changed the title fix: class and style attributes not recognized on components in vue 2.7 test: add tests for Vue 2 Oct 17, 2023
@so1ve so1ve mentioned this pull request Oct 17, 2023
@xiaoxiangmoe
Copy link
Collaborator

This branch has conflicts that must be resolved.

Let me handle it.

@xiaoxiangmoe xiaoxiangmoe force-pushed the fix/class-vue-2 branch 4 times, most recently from 4ae6123 to 714b54f Compare October 17, 2023 11:52
@johnsoncodehk
Copy link
Member

I'll do a more thorough refactoring for vue-tsc test, but let's merge this first. Thanks!

@johnsoncodehk johnsoncodehk merged commit 9bedf4b into vuejs:master Oct 22, 2023
3 checks passed
@rchl rchl deleted the fix/class-vue-2 branch October 22, 2023 19:33
Comment on lines +11 to +21
<script lang="ts">
import { defineComponent } from 'vue';

const Foo = defineComponent({
props: {
foo: String
}
});
</script>
<script setup lang="ts">
</script>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is a bit different from how I wrote this in my changes. My version only had one script setup block.

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 this pull request may close these issues.

None yet

4 participants