mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

mathjs/number missing typescript definitions.

Open nicklasisraelsson opened this issue 2 years ago • 16 comments

There is still no "mathjs/number" typescript definition.

import {
  create,
  all
} from "mathjs/number";

results in typescript error

Cannot find module 'mathjs/number' or its corresponding type declarations.ts(2307)

There was an issue on this before (#1665). It got closed with reference to #1539. #1539 got closed with a comment asking us to open new issues if there is some issue with the current types. So here I am.

nicklasisraelsson avatar Mar 28 '22 10:03 nicklasisraelsson

Good point, thanks Nicklas.

Anyone able to help writing the TypeScript definitions? Help would be welcome.

josdejong avatar Mar 28 '22 10:03 josdejong

I am happy to take a stab at getting the ball rolling by filtering the existing index.d.ts in the types directory if someone can tell me what the path for the number typescript definition file should be. types/number/index.d.ts? types/number.d.ts? number/types/index.d.ts? As you can see, I am really just guessing here... javascript/typescript interoperation is not really something I know about.

gwhitney avatar Apr 01 '22 07:04 gwhitney

Thanks!

I do have no idea either. It would make sense to me to to use the same directory structure and put it in types/number/index.d.ts. What counts though is whether TypeScript can automatically find and use it. That probably involves just trying it out and see if TypeScript is happy when consuming the library (and if not, move the file around until it is).

josdejong avatar Apr 01 '22 09:04 josdejong

Right but I am not even sure how to "try typescript" in the context of a cloned version of the repo -- just how to use npm install mathjs in a typescript project but I can't exactly publish new versions of mathjs just to test this. In other words, I don't know how to get a typescript project into the state it would have been with npm install mathjs but with a version of mathjs that exists only in my cloned source repo.

gwhitney avatar Apr 01 '22 13:04 gwhitney

To test this from a TypeScript project should more or less look like:

  • create a TS project:
    • create a package.json there (npm init)
    • add a tsconfig.json file
    • install mathjs (with the additional TS definitions)
  • create a typescript file myTest.ts, and import { add } from 'mathjs/number' there, try to actually use something a function like add or so.
  • install the typescript compiler on your system
  • see if TypeScript understands the types by running the compiler tsc on the file

The following getting started looks helpful to me: https://www.jonthenerd.com/2021/10/24/getting-started-with-node.js-and-typescript/

josdejong avatar Apr 01 '22 14:04 josdejong

  • install mathjs (with the additional TS definitions)

It's this step I don't understand. I only know how to install mathjs in another project from npm, not from my cloned source repo...

gwhitney avatar Apr 01 '22 14:04 gwhitney

If you have the updated version of mathjs built locally on your computer at say C:\projects\mathjs, you can go to the test project and do npm install C:\projects\mathjs 😁

josdejong avatar Apr 01 '22 14:04 josdejong

OK, it appears from my experiments that number/types/index.d.ts is the place these declarations need to be. So if I create a first-pass version of this file and put it in that location (which is within a new directory and subdirectory for the mathjs project), when everything is built and packaged up and published to npm and so on, will it show up in what clients get when they install mathjs? Or does something need to be added somewhere in package.json as well? Sorry I am so inexperienced with packaging JavaScript projects.

gwhitney avatar Apr 03 '22 19:04 gwhitney

Oh, and as far as I can tell the same declaration file also needs to be in lib/esm/number.d.ts (because it seems that tsc sometimes picks up the type declarations different ways). So if I put the authoritative file in number/types/index.d.ts, where do I put a cp command or something like that to make sure a copy ends up in lib/esm/number.d.ts?

gwhitney avatar Apr 04 '22 03:04 gwhitney

OK, it appears from my experiments that number/types/index.d.ts is the place these declarations need to be.

Sounds good!

will it show up in what clients get when they install mathjs?

Yes indeed. You can verify yourself locally by first running npm run build and then npm pack. The latter will create a zip file that corresponds exactly with what will be published on npm. After publishing, you can also double-check what is inside the npm package for example via unpkg: https://unpkg.com/browse/mathjs/

Oh, and as far as I can tell the same declaration file also needs to be in lib/esm/number.d.ts (because it seems that tsc sometimes picks up the type declarations different ways)

O, I wasn't aware of that. So far the package has been published with the type definitions only in types/..., not a copy in lib/esm. I haven't heard of people having issues with that, but if there is a copy needed in lib/esm/ we should create a new copy script in gulpfile.cjs for that.

josdejong avatar Apr 04 '22 16:04 josdejong

Well the exports in the package.json seem to want to support both import * as math from 'mathjs.number'; and ... from 'lib/esm/number' and I just couldn't seem to find a single location that made both work, regardless of what I put in package.json. Basically it seems fine using the "types" designation for the main import, but it seems to fall back to other resolution methods for other imports from the package. Maybe a better expert could find the configuration that would work with just one copy. But as for me so I don't keep spending lots of time on this, if you don't mind my making one copy of the number typescript declaration, that's what I will do to get something checked in.

gwhitney avatar Apr 05 '22 14:04 gwhitney

argh. I can give it a try, see if I can make TypeScript happy. Please let me know if I can help as soon as you have a first version of the PR ready.

josdejong avatar Apr 05 '22 16:04 josdejong

Ok i will file a WIP PR for this with the copy in place and then you can see if you can clean up my mess ;-)

gwhitney avatar Apr 05 '22 16:04 gwhitney

😂👍

josdejong avatar Apr 06 '22 07:04 josdejong

Hm look like it still doesn't work after #2569

❯ npm why mathjs
[email protected]
image

thien-do avatar Jun 10 '22 09:06 thien-do

Yes, this latest report from @thien-do agrees with my experiments that I needed the number.d.ts file in two different places for it to always be picked up by different ways of importing the library. And incidentally, PR #2569 also set the type declaration file for the number version of mathjs to be the same as for the full version of mathjs, which is definitely not right -- it needs to be a separate version filtered to only support numbers. I think both of these points will need to be addressed by another PR.

gwhitney avatar Jun 10 '22 17:06 gwhitney