-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
[RFC] feature(transformer): Add function overload support #303
base: master
Are you sure you want to change the base?
[RFC] feature(transformer): Add function overload support #303
Conversation
Hi :). I had a quick look! Really great work! First of all I would like to review the strict null check then we can have a look at this PR. I am also concerned about the impact on non supported values for overload methods. I would divide the test in its own file as well. |
00902c1
to
b3a4613
Compare
38a6b79
to
236184f
Compare
Hi, there are few things we could do to improve the readability of this branch
I was thinking that we may want to add a features sections where you can pass strings for example interface TsAutoMockOptions {
debug: boolean;
cacheBetweenTests: boolean;
features: string[];
} This will give as the advantage to add more features in the future in the same block.
What do you think? |
I'm with you on that one, but I had to include things from the other repos to get tests to pass :) One neat thing you can do though, is to pick a different range to diff in the "Changed files" tab, excluding the commits from the depending branch (PR). It's not an ideal solution, but it does help a little. It was perhaps a bit early to review this, sorry - I haven't gotten around to push a refactor of the branching logic, it does clean things up a bit.
Sounds like a good idea. However, I'd prefer to have the list of features be typed as well if so. But, ...
I'm not exactly sure why you want this plug-n-play feature. I'd expect to have everything enabled in a TDD-environment and not worry so much about performance, since things will be implemented as you go and the overhead of performance decreases as you progress. Maybe that's just me.
I was actually thinking of this myself and I certainly think we should cover both scenarios. If the feature-gate is the way we'll approach this I suggest we test all permutations of features. And we may have to adjust the current test setup/CI accordingly. |
…fferentiate them in conditional typing
236184f
to
a06ff79
Compare
I will take a look at documentation tomorrow :) |
…w for overloads with non literal inputs
…k calls of interfaces with call signatures
… and utilize TypeScript's MethodSignature type
…hod signature-based hash
…entifier of a BindingName E.g.: ``` const [[[[[var]]]]] = ... ```
…ed methods as well
a06ff79
to
6eefd02
Compare
I've come up with a completely different approach to this, see: martinjlowm@340b562 It works much better and is far simpler and it doesn't have all the problems than the branching logic implementation has per argument. This new approach scales linearly with the number of overload signatures, rather than the exponential scaling of this PR (num. signature times num. inputs) cc: @uittorio |
edit: After a recent (write-up) discussion (see https://www.notion.so/RFC-Overload-c5a832ca51654f58a5019bf5507013a7), this PR is on hold. It'll add too much complexity in its current state and the project is not ready for such big changes. Feel free to leave a comment here if you have ideas or proposals for how we can approach this.
edit: Moved this PR into draft. It's dependent on a solution to the issue discussed in #313
edit: This now depends on #313 and #314
As the title states, this PR adds support for declared function overloads for (union) non Object-inherited types through the new option,
transformOverloads
, which is turned off by default. Turning this functionality off, enforces the transformer to select the first signature which is the same behavior as previously.The runtime branching follows that of the TypeScript type checker, meaning the first overload has precedence over those that follow.
The signature of
GetMethodDescriptor
has been changed to accept an array of signatures that specifies parameter declarations and a return value.Here's a snippet of how the transformation looks:
This is applied to overloads in interfaces and of declared functions.
Currently, only non Object-inherited inputs are supported and the changes in this PR will fall back toinstanceof Object
if such arguments are provided with a warning that states so.