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

Type declarations do not understand typed-function's automatic conversions #2582

Open
gwhitney opened this issue May 30, 2022 · 8 comments
Open

Comments

@gwhitney
Copy link
Collaborator

gwhitney commented May 30, 2022

For example, one signature of math.exp is Complex. But there is a conversion in mathjs from Fraction to Complex. Hence,
math.exp(math.fraction(1,1)) is a legal expression that returns e + 0i. On the other hand, currently in typescript
const complexe: Complex = math.exp(math.fraction(1, 1)) produces:

index.ts:142:40 - error TS2769: No overload matches this call.
  Overload 1 of 3, '(x: number): number', gave the following error.
    Argument of type 'Fraction' is not assignable to parameter of type 'number'.
  Overload 2 of 3, '(x: BigNumber): BigNumber', gave the following error.
    Argument of type 'Fraction' is not assignable to parameter of type 'BigNumber'.
      Type 'Fraction' is missing the following properties from type 'Decimal': e, toStringTag, absoluteValue, abs, and 97 more.
  Overload 3 of 3, '(x: Complex): Complex', gave the following error.
    Argument of type 'Fraction' is not assignable to parameter of type 'Complex'.
      Type 'Fraction' is missing the following properties from type 'Complex': re, im, clone, equals, and 6 more.

This example is of course just the tip of this iceberg (there are many other automatic conversions). It does not seem to me that a reasonable solution is to go through all of the functions in index.d.ts and add the legal signatures resulting from the conversions. But I am not sure what a reasonable approach would be. I imagine one could define a type ComplexOrConvertibleThereto (needs a better name) and change the parameters. But that can't be a simple search-and-replace, since for those functions that have a Fraction signature as well as Complex, the separate Fraction definition takes precedence over the one that would come from converting Fraction to Complex

@josdejong
Copy link
Owner

Thanks for bringing this up. Looking at the definition of exp for example, it looks like:

    /**
     * Calculate the exponent of a value. For matrices, the function is
     * evaluated element wise.
     * @param x A number or matrix to exponentiate
     * @returns Exponent of x
     */
    exp(x: number): number
    exp(x: BigNumber): BigNumber
    exp(x: Complex): Complex
    exp(x: MathArray): MathArray
    exp(x: Matrix): Matrix

This looks well defined, only, Fraction is missing. I see Fraction is missing for many functions. I think there is not directly a need to solve this in a generic way or so, but we can just go the dumb way and add type definitions for Fraction where missing. Anyone able to help out here?

@gwhitney
Copy link
Collaborator Author

I respectfully disagree; I was giving just one example. There are numerous conversions. For example, string converts to number. Do we really want to go through by hand and add a string signature to almost every function in index.d.ts?

@josdejong
Copy link
Owner

I understand your point. Right now, mathjs is plain JavaScript, and the TypeScript definitions do not fully cover mathjs. They come "close". It is not trivial to rewrite mathjs in TypeScript (though I would like to take that step). So until then, we simply have to deal with the current situation. It will not be possible to make the types 100% right. I guess there is a sweat spot somewhere that we have it 80% or 90% correct with "relatively little effort". Trying to get towards 100% will cost so much effort that it is not worth it.

Improving the types for Fraction the dumb way by writing the types by hand will probably take 15-30 minutes, so I guess that is worth it. The more subtle and complex conversions and differing behavior depending on config options like predictable is probably too much. What do you think?

@mattvague
Copy link
Contributor

mattvague commented Jun 8, 2022

I understand your point. Right now, mathjs is plain JavaScript, and the TypeScript definitions do not fully cover mathjs. They come "close". It is not trivial to rewrite mathjs in TypeScript (though I would like to take that step). So until then, we simply have to deal with the current situation. It will not be possible to make the types 100% right. I guess there is a sweat spot somewhere that we have it 80% or 90% correct with "relatively little effort". Trying to get towards 100% will cost so much effort that it is not worth it.

@josdejong as I understood on our call, the biggest (or first?) hurdle for converting the codebase to TS would be trying to get https://github.com/josdejong/typed-function to work with Typescript, is that right? If so, I have a few different ideas and would be happy to start spiking on that a bit.

Some references / possible starting points:

@mattvague
Copy link
Contributor

Is there already a task / discussion for the TS conversion?

@josdejong
Copy link
Owner

Yes correct, first step would be to add TypeScript support to typed-function. There is no single TypeScript topic discussing though we've had discussions on it here and there. Let's use #2076 to further discuss it ok?

@gwhitney
Copy link
Collaborator Author

gwhitney commented Jun 9, 2022

Thanks for moving the discussion on converting mathjs to TypeScript elsewhere, as this issue seems to be primarily about being able to correctly use mathjs from TypeScript. And in response to that, I just wanted to say that as far as I can see, even ignoring features like the 'predictable' parameter (which we could just ban the use of from TypeScript), the current set of signatures isn't anything like 90%. It's like 90% if you ignore all type conversions -- the TypeScript declarations seem to generally be derived from the calls to typed, which of course do not mention all of the convertible types, that's the point of automatic conversion. But there are a dozen or more automatic conversions in mathjs, and the vast majority of typed functions in mathjs potentially trigger one or more of them, so taking those conversions into account, the current typescript signatures are more like 10% correct (although in practice a very useful 10%!).

I'll be frank, I think the best way forward here would be to extend typed-function to allow return-type annotations as well (which has other uses and would also be an incremental step toward converting to TypeScript anyway) and then add a method to a typed function which emits a valid TypeScript declaration for itself. Then we could ditch (most of) the current index.d.ts and just generate it at build time.

Short of that, the only method I see for getting index.d.ts close to right is to define types like 'ConvertibleToNumber' which is a union of number and all types for which there is an automatic conversion to number and use them in place of 'number' in most instances in index.d.ts (and the same for all other types which are the destinations of conversions) and try it and see if it all works out.

@josdejong
Copy link
Owner

the current typescript signatures are more like 10% correct

ha ha now you almost make me cry 😉

Agree, types in typed-function should be addressed first. And then we need a solution to auto-magically generate all these types (including based on conversions etc) for us. That will still not address things like differing behavior based on config like predictable, but at least get us much further than 10%.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants