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

Export all TS types used by public API #2020

Closed
Raiondesu opened this issue Aug 31, 2020 · 10 comments
Closed

Export all TS types used by public API #2020

Raiondesu opened this issue Aug 31, 2020 · 10 comments

Comments

@Raiondesu
Copy link

Raiondesu commented Aug 31, 2020

What problem does this feature solve?

Let's say, there's a situation sometime that requires a user of Vue 3 to make a certain wrapper or extension for the standard public API.

For example...

Currently, there's only one way to define props for a functional component, as stated in docs:

import { h } from 'vue';

// extended to a more realistic example
// here, in TS, a user has to define props separately in two places, which produces code duplication
const DynamicHeading = (props: { level: number }, context) => {
  return h(`h${props.level}`, context.attrs, context.slots);
};

DynamicHeading.props = {
  level: Number;
};

export default DynamicHeading;

This is far from perfect for user experience, and is definitely in need of some sort of a wrapper (with type inference, preferably).

Something like

import { h } from 'vue';

// No code duplication whatsoever!
export default withProps({
  level: Number;
}, (props, context) => {
  // here props.level is already defined
  return h(`h${props.level}`, context.attrs, context.slots);
});

The defineComponent function is public and uses some typings to provide type information for the component and props alike, which would also be useful for this wrapper.

I've written this wrapper myself for my project only, but have encountered an issue - currently most of the types of @vue/runtime-core are internal and are not exported, therefore making a job of creating such wrappers practically an impossible choice.
So, now you either:

  • do not write any wrappers at all and live with boilerplate code, which is painful;
  • or practically copy over half of @vue/runtime-core/dist/runtime-core.d.ts into the project, because most of the types used in defineComponent are not exposed.

So, with all types exported from d.ts files, the end user gains the ability to more freely understand and expand upon the standard Vue 3 API without the need to write boilerplate code or copy half of the typings into their own project.

The wrapper is still possible to create without vue typings, it's just an example. But it still would lack some typing clarity the internal types could provide...

What does the proposed API look like?

Export all types without exceptions from d.ts files so users can more freely extend and compose current API properly (this was the whole point from the start, right?)...

This can be done with existing APIs, as no new APIs need to be added - just a couple of exports for compile-time types here and there.

I get that there are some types that are used only by internal APIs that are not exposed, and it may be okay.
But there are types used by public APIs (like defineComponent), that are still not exposed.
So, not exposing types used by public APIs is like not exposing some part of the already exposed API.

In my opinion, exposed types allow for:

  • a more cohesive substitution techniques - user doesn't have to fork Vue to replace any part of its API - a couple of imported types is enough to maintain guaranteed type symmetry and LSP;
  • ability to fix bugs in-place without the need to wait until Vue team fixes them or approves a fix (and without forks);

P.S. If this gets any positive feedback - I'll gladly propose a PR myself.

P.P.S. The issue itself is more about the lack of type exports than the wrapper itself - it's provided as more of an example of what's to come (in terms of user experience problems) when Vue 3 makes it to production and is adopted by industry-scale projects.

@Raiondesu Raiondesu changed the title Export all TS types without exceptions (or provide a functional way of handling props in functional components) Export all TS types without exceptions Aug 31, 2020
@yyx990803
Copy link
Member

You already have the types needed to write that function.

import { FunctionalComponent, ExtractPropTypes } from 'vue'

function withProps<T>(props: T, fn: FunctionalComponent<ExtractPropTypes<T>>) {
  // ...
}

@Raiondesu
Copy link
Author

Ok, thank you very much for such a nice response.

But what about other use-cases I personally couldn't come up with?

@Raiondesu
Copy link
Author

Still, for the exact use-case I provided, current types provide some weird results:
image
Props here are definitely required, but are still marked as optional in the type, with no obvious way around this, except for maybe iterating over props and conditionally checking each one, like this:
image
even though the types for ExtractPropTypes seem to be correct.

Oh, and array props don't work at all this way:
image


But that's not my point.

As stated in the original issue post, my point is that there are always types that need to be exposed, but aren't.

And fulfilled requests for exposing some particular types keep piling up in the changelog (10+ cases as of time of this writing), because it turns out at some point that some types are needed to test/extend/use the API.
Just like it was with ExtractPropTypes itself.

@pikax
Copy link
Member

pikax commented Sep 1, 2020

Still, for the exact use-case I provided, current types provide some weird results:
image

You need to cast as const because the required gets the type of boolean instead of true.

withProps(
  {
    msg: {
      required: true,
      type: String
    }
  } as const,
  props => {
    props.msg
  }
)

As stated in the original issue post, my point is that there are always types that need to be exposed, but aren't.

Which types that you need that need to be exposed? There's a lot of internal types that should not be exposed.


Please note for questions please use the forum or discord.

@Raiondesu
Copy link
Author

Raiondesu commented Sep 1, 2020

Hey, @pikax, thanks for the answer.

Can you, please, make a somewhat convincing case on why "internal types" should not be exposed?
It's not like they're some executable pieces of code that could "break" Vue in the hands of users, like some internal runtime APIs could.

I get that there are some types that are used only by internal APIs that are not exposed, and it may be okay.
But there are types used by public APIs (like defineComponent), that are still not exposed. So I'm saying that there is a high probability that there will be a point in time, eventually, when types for exposed APIs are needed somewhere. It could even be as simple as TS failing to inference types in very large projects (a known issue of TS).
So, not exposing types used by public APIs is like not exposing some part of the already exposed API. Like giving type information about some methods in a class, but not others (assuming all are marked as public).

In my opinion, exposed types allow for:

  • a more cohesive substitution techniques - user doesn't have to fork Vue to replace any part of its API - a couple of imported types is enough to maintain guaranteed type symmetry and LSP;
  • ability to fix bugs in-place without the need to wait until Vue team fixes them or approves a fix (and without forks);

One could argue about breaking changes in internal typings. But then, if such a situation occurs on types that are used in public API (like defineComponent), then that public API is also affected by this change, so it's no matter if the type is exposed or not - the breaking change in internal types will still hit the user.
By exposing types used by public APIs, we're being honest about what's acutally public and cannot change vs what's internal and can change. This could prevent a lot of accident type-related breaking changes in the future.

I think I didn't quite state it right in the first place, that I'm only talking about types that are used by public APIs. I'll edit the original post to better represent this point.


Which types that you need that need to be exposed?

For example, the PropOptions interface is exposed in 2.x, but is internal in 3.x (though it didn't change much between the versions).
My guess is that it's not the only one like this, though I didn't research this enough yet.
Isn't this considered a breaking change - when a new version hides APIs that are potentially used by users?

My project that I'm migrating to 3.x is heavily dependent specifically on this type. So there's no other choice for me right now except to just copy it to my d.ts files, as the exposed Prop<T> has extra type information that I don't need.


Please note, that I'm not having troubles or asking for help regarding Vue types.
I'm concerned about the API being not fair to end user about it's publicity.

@Raiondesu Raiondesu changed the title Export all TS types without exceptions Export all TS types used by public API Sep 1, 2020
@pikax
Copy link
Member

pikax commented Sep 1, 2020

Can you, please, make a somewhat convincing case on why "internal types" should not be exposed?

IMO is to make the vue types bloated with unnecessary types, this will prevent the user to try to access internal APIs or write code with a type that might change. This simplify users life with less types to be import, such as ComponentOptionsWithoutProps | ComponentOptionsWithArrayProps | ComponentOptionsWithObjectProps | etc , instead there's only one Component.

You haven't given any good arguments on why "all" types should be exported.

But there are types used by public APIs (like defineComponent), that are still not exposed

Such as?
Improving defineComponent is under works

In my opinion, exposed types allow for:

  • a more cohesive substitution techniques - user doesn't have to fork Vue to replace any part of its API - a couple of imported types is enough to maintain guaranteed type symmetry and LSP;
  • ability to fix bugs in-place without the need to wait until Vue team fixes them or approves a fix (and without forks);
  • What substitution? if the "type" is an interface you can already do that in the userland.
  • I don't think this is a good argument. If you want to patch "interfaces" you can do it already, exporting types wouldn't change anything.

For example, the PropOptions interface is exposed in 2.x, but is internal in 3.x (though it didn't change much between the versions).

You should be using the Prop type.

My project that I'm migrating to 3.x is heavily dependent specifically on this type.

That's exactly the reason you shouldn't be accessing "internal" types, vue2 types are different from vue3, there's works to bring them closer.

Please note, that I'm not having troubles or asking for help regarding Vue types.

You example and the subsequent reply to the solution provided, shows that you are having troubles with the vue types,

I'm concerned about the API being not fair to end user about it's publicity.

Which types that you need that need to be exposed?


If you still need help extracting the types, please use #typescript on discord, there's a lot of capable ppl there.

Just to make it clear, "export all TS types" is not correct, export specific types which are not being currently exported is still open for discussion. If you have a specific type (which you don't have access currently), you can create a PR to export it, it will be discussed case by case.

If you still not happy with my response, please create an RFC to bring this discussion to the community.

@Raiondesu
Copy link
Author

Raiondesu commented Sep 1, 2020

Ok, thank you for a very detailed reply.


Just to make it clear, "export all TS types" is not correct

I agree with this specific statement, which is why I've already corrected my point to "export all TS types used by public API" that better reflects my intentions and concerns.


But there are types used by public APIs (like defineComponent), that are still not exposed

Such as?

ComponentPublicInstanceConstructor, CreateComponentPublicInstance, InferPropType, IntersectionMixin, PropConstructor, PropMethod, UnmountChildrenFn, DevtoolsHook, VNodeNormalizedRef to name a few in a single file.

All of them have a couple of things in common:

  • All used by an exposed/public function or type in the top level (directly in parameters, in generics' type guards or otherwise directly used).
  • All of exposed/public functions or types use them alongside exposed types (VNodeNormalizedChildren is exposed, but VNodeNormalizedRef isn't - both used in exposed VNode, for example).
  • There are internal APIs that use them alongside with external APIs.
  • There are types that are labeled ...Internal..., that are exposed and use these not exposed types.

You should be using the Prop type.

As I stated, it has extra type information that I don't need.


You haven't given any good arguments on why "all" types should be exported.

I didn't try to, because I was only giving arguments on why "all types of public APIs" should be exported.


What substitution? if the "type" is an interface you can already do that in the userland.

Not with public APIs that use internal types.
It's not possible to competently substitute defineComponent or h, for example, without basically copying over half of runtime-core.


You need to cast as const because the required gets the type of boolean instead of true.

 withProps(
    {
      msg: {
        required: true,
        type: String
      }
    } as const,
    props => {
      props.msg
    }
  )

This is syntactic burden in userland, can be avoided by using Readonly (only for object, though), as stated in Vue sources.


Your example and the subsequent reply to the solution provided, shows that you are having troubles with the vue types.

I've already come up with a solution myself, I just wanted to prove a point.

@binhonglee
Copy link

I found out this thread in the process of migrating my project from Vue 2 to Vue 3. In Vue 2, I've used Vue type for passing around a component as a param but now that CreateComponentPublicInstance isn't exported as a type (returning type of defineComponent()), I can no longer do the same. As a workaround, I'm temporarily using any as a placeholder but I feel like it kinda defeats the point of using TypeScript (or that having a typed framework) when I can't inherit those predefined types.

@pikax
Copy link
Member

pikax commented May 9, 2021

I found out this thread in the process of migrating my project from Vue 2 to Vue 3. In Vue 2, I've used Vue type for passing around a component as a param but now that CreateComponentPublicInstance isn't exported as a type (returning type of defineComponent()), I can no longer do the same. As a workaround, I'm temporarily using any as a placeholder but I feel like it kinda defeats the point of using TypeScript (or that having a typed framework) when I can't inherit those predefined types.

@binhonglee defineComponent returns DefineComponent which contains all the component information, you can extract the public instance by

const Comp = defineComponent({ /* component definition */}

type CompTypeDefinition = typeof Comp; // should be DefineComponent 

type CompInstance = InstanceType<CompTypeDefinition>

CreateComponentPublicInstance Is an internal type and only use to generate a PublicInstance based on options, I don't know a reason to be used on the user land (please let me know your use case).

@binhonglee
Copy link

binhonglee commented May 9, 2021

I found out this thread in the process of migrating my project from Vue 2 to Vue 3. In Vue 2, I've used Vue type for passing around a component as a param but now that CreateComponentPublicInstance isn't exported as a type (returning type of defineComponent()), I can no longer do the same. As a workaround, I'm temporarily using any as a placeholder but I feel like it kinda defeats the point of using TypeScript (or that having a typed framework) when I can't inherit those predefined types.

@binhonglee defineComponent returns DefineComponent which contains all the component information, you can extract the public instance by

const Comp = defineComponent({ /* component definition */}

type CompTypeDefinition = typeof Comp; // should be DefineComponent 

type CompInstance = InstanceType<CompTypeDefinition>

CreateComponentPublicInstance Is an internal type and only use to generate a PublicInstance based on options, I don't know a reason to be used on the user land (please let me know your use case).

My current use case is mainly to get input element values through refs or to get the element itself to set focus. As far as I know, refs are still not strictly type aware so it requires a lot of typecasting and repeating it by hand is a little tedious. So I made some sort of a helper class to deal with that.

Your suggestion seems to only apply if I know the specific component I'll be passing around instead of generically any Vue component. I tried using DefineComponent but I believe vue-tsc complains about incompatible type when calling the function from within the component with this.

Edit: Also seems like DefineComponent only have access to ref as opposed to $refs.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants