bennycode / trading212-api

https://bennycode.com/trading212-api
MIT License
0 stars 1 forks source link

Problem with types, could possibly fix by definately typed definitions or suggestion on tsconfig settings #15

Closed max-carroll closed 2 weeks ago

max-carroll commented 2 weeks ago

Hi There,

Wonderful library by the way, I wanted to play around with trading 212 and was going to make a client library, but it seems you've already made one so thanks!

The types don't seem to be working out of the box, image

Doesn't seem to be a definitely typed package available for this, should I need one,

I am going to play around with the TS Config until I get this working, as I think I can just add types or typeRoots, once I have a solution I can create a PR for your getting started guide if you like, unless I've done something fundamentally wrong (which is possible)

bennycode commented 2 weeks ago

Hi @max-carroll, let me check this. I will fix it for you today.

max-carroll commented 2 weeks ago

Does it need the types to be concatenated into one file and then this referenced in the "types" property in the package.json?

Just had a look at what @mui do

max-carroll commented 2 weeks ago

Hi Benny, I have managed to fix this locally with a few changes, I pointed my app consuming your library to the local version of your trading212-api which I checked out , I then made the following changes

image using this article as a guide https://digitalfortress.tech/tips/link-npm-package-with-its-types/

Sorry if your familiar with this, my intention was just to inform you if you didn't know

I had to change some settings to get squggly lines do disappear in vscode, it was unhappy about using top level async unless I changed the module and target to something, but then I got a build error when trying to build with the declaration files unless I used NodeNext that seems to make everything shut up :D

bennycode commented 2 weeks ago

Hey @max-carroll and thank you for doing all of this.

The error is actually quite simple. The "main" entry in my "package.json" file is set to "dist/index.js": https://github.com/bennycode/ts-node-starter/blob/v1.0.0/package.json#L40

BUT there is no "dist/index.js" file in the uploaded package on npm. There only is a "dist/src/index.js" file: https://www.npmjs.com/package/trading212-api/v/1.0.2?activeTab=code

Because of this wrongly configured "main" entry, the TypeScript compiler cannot find the "dist/src/index.d.ts" file which it needs to successfully inspect the typings.

I will fix that.

max-carroll commented 2 weeks ago

Ah right, thanks Benny!

max-carroll commented 2 weeks ago

Well spotted, I didn't notice that

bennycode commented 2 weeks ago

Thank you. I spent a good amount of time with TypeScript to build TypeScript TV. 😅

This shouldn't have slipped through the cracks. I will add the @arethetypeswrong/cli package to make sure this doesn't happen in the future. This tool will flag broken "main" entry points like this:

image

bennycode commented 2 weeks ago

@max-carroll I have added the attw safeguard to prevent such kind of errors in the future: https://github.com/bennycode/trading212-api/commit/73115ca9b40ac35283d7908977371526fcb2ec97

I also made a new release (v1.1.0). Can you install it and try again?

max-carroll commented 2 weeks ago

Yes it seems to be working now,

To confirm this I rimrafed the node_modules forlder and restarted VSCode TS Server to observe VSCode complaining about not being able to find the module or types declaration.

I then installed the new version, to observe that it is fixed.

Thanks for your very fast turn around on fixing the issue

bennycode commented 2 weeks ago

You are welcome! Happy to help. :)