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

TypeScript-specific feedback & thoughts #3

Closed
DanielRosenwasser opened this issue Oct 4, 2018 · 6 comments
Closed

TypeScript-specific feedback & thoughts #3

DanielRosenwasser opened this issue Oct 4, 2018 · 6 comments

Comments

@DanielRosenwasser
Copy link
Contributor

DanielRosenwasser commented Oct 4, 2018

I'd like to keep the other breaking changes feedback thread scoped to breaking changes. Here's some TypeScript-specific feedback which doesn't need to be addressed immediately, but which stood out to me while exploring the package.

Generic type parameter for props (P) should come before data (D)

This was the biggest footgun I made when I worked on the Vue .d.ts files the first time. I specified Props first rather than Data and I believe that Vue 3.0 shouldn't make the same mistake. When it comes to components, almost all of them will use props and have a harder time specifying them than data.

Generic type parameter P has defaults

There are a lot of instances of P = Data (which is really just P = Record<string, any>). Is there any reason that's necessary? I feel like the class-based API doesn't really need this since, on the whole, it's mostly inferred, though I could be missing something.

Vue is currently non-generic

Vue (from packages/vue) currently has a different API than Component (from packages/core). Specifically, Vue is generic and seems to be the "full-featured" version of the component. You already mentioned that things are in a state of progress, but is the difference mostly about mount points depending on target runtimes (i.e. DOM vs. native vs. ...)?

.d.ts files are produced using dts-bundle

dts-bundle throws all of your modules into a single global file with several ambient module declarations. The problem with ambient module declarations is that they are in the global scope in the first place. If you have multiple versions of Vue loaded up by a project (via a separate dependency probably), then two ambient modules for the path @vue/renderer-dom can conflict, which can cause problems.

Mitigations

We should consider looking into API Extractor to produce .d.ts files that describe top-level APIs for Vue.

Build process could leverage project references.

Right now it appears that there's a custom TypeScript build that happens, and it seems like that's an all-or-nothing approach. Depending on whether build times are an issue, we could potentially leverage project references. If there's a reason you can't use them, it'd be helpful for our team to get an idea of the challenges. 😃

@yyx990803
Copy link
Member

yyx990803 commented Oct 4, 2018

Generic type parameter for props (P) comes before data (D)

I'm a bit confused because currently D comes before P. Are you suggesting that Props should come before Data, or the other way around?

Generic type parameter P has defaults

I guess it should default to {} instead. I gave it the Record<string, any> default mostly so that I can omit the generics and get a magical this when I don't want to bother with specifying types. Changing to to {} would force users to always fully type their props. It may be a bit too strict but maybe that's a good idea.

Vue is currently non-generic

Yeah this part is very much just a proof of concept right now. We will revisit proper typing for it when we finalize it. The vue package will likely be just the 2.x compat build which will use the 2.x typings...

.d.ts files are produced using dts-bundle

Good to know, I wasn't aware of API extractor before. Will look into it.

Build process could leverage project references.

I'm not sure if I understand the proposal - I haven't seen the need for project references so far. Maybe we will need it later? Are you suggesting we place a tsconfig.json in each package and reference the root tsconfig.json?

@DanielRosenwasser
Copy link
Contributor Author

DanielRosenwasser commented Oct 5, 2018

Generic type parameter for props (P) comes before data (D)

I'm a bit confused because currently D comes before P. Are you suggesting that Props should come before Data, or the other way around?

Whoops! Fixed. Yes, I meant P should come before D.

Generic type parameter P has defaults

I guess it should default to {} instead. I gave it the Record<string, any> default mostly so that I can omit the generics and get a magical this when I don't want to bother with specifying types. Changing to to {} would force users to always fully type their props. It may be a bit too strict but maybe that's a good idea.

I think that users looking for Vue/TypeScript will prefer the stricter experience. Even so, if we can do some more inference here that will probably strike a better balance of writing less while still getting better types. I've opened up microsoft/TypeScript#27584 so we can track that.

Vue is currently non-generic

Yeah this part is very much just a proof of concept right now. We will revisit proper typing for it when we finalize it. The vue package will likely be just the 2.x compat build which will use the 2.x typings...

That sounds reasonable to me, though I'm wondering if the idea is to just have two separate builds, or if the Vue 2.x functionality is available out of the box.

Build process could leverage project references.

I'm not sure if I understand the proposal - I haven't seen the need for project references so far. Maybe we will need it later? Are you suggesting we place a tsconfig.json in each package and reference the root tsconfig.json?

That's basically it, but I wouldn't get hyper-focused on that if it's not a problem. It can just give you faster builds; if A depends on B depends on C, a change to Cthat doesn't affect its public API means that neitherAnorB` need to be rebuilt if their outputs are built and already up-to-date.

@DanielRosenwasser
Copy link
Contributor Author

DanielRosenwasser commented Oct 5, 2018

props duplication & validation

From #2 (comment)

A few obvious things to improve here:

1. If the user provided a Props interface but didn't specify the static props options, the props will still be merged onto `this` in types but not in implementation.

2. User has to specify the Props interface for compile-time type checking AND the props options for runtime type checking. It'd be nice if the compile-time types can be inferred from the props options like it does for 2.x, although I haven't figured out how to do that yet.

I'm a little curious over what the rationale is to only proxy specified props. What are the current design limitations that prevent this.


Note the reason we are designing it like this is because we want to make plain ES usage and TS usage as close as possible. As you can see, an ES2015 version of the above would simply be removing the interfaces and replacing the static options with an assignment.

If we can get a decorator implementation that matches the new stage-2/3 proposal, we can provide decorators that make it simpler:

class Foo extends Component {
  @data foo: string = 'hello'
  @prop bar: number = 123
}

I agree that this'd be nice, though, the problem here is that now there's nowhere to keep track of props externally. So while you get internal validation, there's not a way to keep track of which props you're aggregating. I'll check with the team to get some thoughts around this.

@DanielRosenwasser
Copy link
Contributor Author

Even so, if we can do some more inference here that will probably strike a better balance of writing less while still getting better types. I've opened up microsoft/TypeScript#27584 so we can track that

Actually, I realized that props are specified statically (as they should be), not via a super() call. So never mind that!

@yyx990803
Copy link
Member

I'm a little curious over what the rationale is to only proxy specified props. What are the current design limitations that prevent this.

When a component doesn't declare props, the consumer of the component can pass arbitrary props to it that may conflict with the component's own data or methods. With declared props we can check duplication and warn at development time.

@yyx990803
Copy link
Member

Closing since we are dropping the Class API.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 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

2 participants