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

mathjs/number missing typescript definitions. #2506

Open
nicklasisraelsson opened this issue Mar 28, 2022 · 16 comments
Open

mathjs/number missing typescript definitions. #2506

nicklasisraelsson opened this issue Mar 28, 2022 · 16 comments

Comments

@nicklasisraelsson
Copy link

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.

@josdejong
Copy link
Owner

Good point, thanks Nicklas.

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

@gwhitney
Copy link
Collaborator

gwhitney commented Apr 1, 2022

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.

@josdejong
Copy link
Owner

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).

@gwhitney
Copy link
Collaborator

gwhitney commented Apr 1, 2022

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.

@josdejong
Copy link
Owner

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/

@gwhitney
Copy link
Collaborator

gwhitney commented Apr 1, 2022

  • 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...

@josdejong
Copy link
Owner

josdejong commented Apr 1, 2022

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 😁

@gwhitney
Copy link
Collaborator

gwhitney commented Apr 3, 2022

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
Copy link
Collaborator

gwhitney commented Apr 4, 2022

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?

@josdejong
Copy link
Owner

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.

@gwhitney
Copy link
Collaborator

gwhitney commented Apr 5, 2022

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.

@josdejong
Copy link
Owner

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.

@gwhitney
Copy link
Collaborator

gwhitney commented Apr 5, 2022

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 ;-)

@josdejong
Copy link
Owner

😂👍

@thien-do
Copy link

thien-do commented Jun 10, 2022

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

❯ npm why mathjs
mathjs@10.6.1

image

@gwhitney
Copy link
Collaborator

gwhitney commented Jun 10, 2022

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.

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

4 participants