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

types: improve readability of built-in type #9129

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Mini-ghost
Copy link

@Mini-ghost Mini-ghost commented Sep 4, 2023

Description

We could write this in code, but the types would be unwrapped:

const date = ref(new Date())

We can declare types that should avoid reference unwrapping by writing:

declare module '@vue/reactivity' {
    Export interface RefUnwrapBailTypes {
        Date: Date;
    }
}

But Date is a commonly used built-in type, would it be better if we support it directly?

Before

const date: Ref<{
    toString: () => string;
    toDateString: () => string;
    toTimeString: () => string;
    toLocaleString: {
        (): string;
        (locales?: string | string[] | undefined, options?: Intl.DateTimeFormatOptions | undefined): string;
        (locales?: Intl.LocalesArgument, options?: Intl.DateTimeFormatOptions | undefined): string;
    };
    ... 39 more ...;
    [Symbol.toPrimitive]: {
        ...;
    };
}>

After

const date: Ref<Date>

@Mini-ghost Mini-ghost changed the title type: better hints for built-in Date types types: improve readability of built-in 'Date' type Sep 4, 2023
@Mini-ghost Mini-ghost changed the title types: improve readability of built-in 'Date' type types: improve readability of built-in Date type Sep 4, 2023
@github-actions
Copy link

github-actions bot commented Sep 4, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 86.9 kB 33.1 kB 29.9 kB
vue.global.prod.js 133 kB 49.9 kB 44.7 kB

Usages

Name Size Gzip Brotli
createApp 48.3 kB 19 kB 17.3 kB
createSSRApp 51.5 kB 20.3 kB 18.5 kB
defineCustomElement 50.7 kB 19.8 kB 18.1 kB
overall 61.7 kB 23.9 kB 21.7 kB

packages/reactivity/src/ref.ts Outdated Show resolved Hide resolved
@Mini-ghost Mini-ghost changed the title types: improve readability of built-in Date type types: improve readability of built-in type Sep 5, 2023
@sxzz sxzz added the ready to merge The PR is ready to be merged. label Sep 5, 2023
@Mini-ghost
Copy link
Author

I deleted BaseTypes because its definition overlaps with Builtin.

Copy link

codspeed-hq bot commented Dec 11, 2023

CodSpeed Performance Report

Merging #9129 will improve performances by 63.31%

Comparing Mini-ghost:type/support-date-type (57dc127) with main (9d1ca32)

Summary

⚡ 1 improvements
✅ 52 untouched benchmarks

Benchmarks breakdown

Benchmark main Mini-ghost:type/support-date-type Change
reduce *readonly* array, 10 elements 1.8 ms 1.1 ms +63.31%

@yyx990803
Copy link
Member

I think you misunderstood what I meant - BuiltIn should include Primitive. This PR shouldn't alter how Builtin works.

That said my last requested change seems unnecessary, since UnwrapRefSimple is only checking for object / collection types so your previous implementation was actually ok and we should revert to that one. Sorry for the confusion!

@Mini-ghost
Copy link
Author

Understood! I will revert back to the previous commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge The PR is ready to be merged.
Projects
Status: Waiting
Development

Successfully merging this pull request may close these issues.

None yet

4 participants