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

Support typescript #108

Merged
merged 4 commits into from
Oct 1, 2023
Merged

Support typescript #108

merged 4 commits into from
Oct 1, 2023

Conversation

woodser
Copy link
Owner

@woodser woodser commented Oct 14, 2022

Supersedes #93

To test this PR:

  1. ./bin/build_dist.sh
  2. npm run test -- --grep "Sample code demonstration" which runs TestSampleCode.ts

@Janaka-Steph
Copy link

The intention is to emit types but keep JS files? Why not converting every files to TS?

@woodser
Copy link
Owner Author

woodser commented Jan 11, 2023

The intention is to emit types but keep JS files? Why not converting every files to TS?

The library files themselves are written in JS, not TS. Are you suggesting to rewrite the library files in native TS?

In addition to being a huge effort, it shouldn't be necessary, since the final types are emitted.

But I may be missing something and I'm open to any suggestions. Thanks.

@Janaka-Steph
Copy link

But you don't benefit from type checking when types are separated like that. It's only useful for the consumer right?

@woodser
Copy link
Owner Author

woodser commented Jan 11, 2023

But you don't benefit from type checking when types are separated like that. It's only useful for the consumer right?

Yes it's only useful for the consumer. Ideally the entire library would be typescript, I think.

@Janaka-Steph
Copy link

Yes would be better. Maybe can try this tool or similar to do the boring work and then replacing with better types little by little https://github.com/airbnb/ts-migrate

@Janaka-Steph
Copy link

@woodser
Copy link
Owner Author

woodser commented Aug 7, 2023

Rebased on master.

I think a good next step is to start renaming .js files to .ts and add typing information to the method signatures.

@Janaka-Steph
Copy link

@woodser woodser removed the help wanted Extra attention is needed label Sep 17, 2023
@hundehausen
Copy link

Hey @woodser are you planning on renaming the project to monero-typescript, when this project is done? :D

@woodser
Copy link
Owner Author

woodser commented Sep 18, 2023

I was considering that, or maybe monero-ts? :)

@hundehausen
Copy link

monero-ts is a great choice!

@everoddandeven
Copy link

Hi, I'm also working on a file conversion from javascript to typescript. Maybe I can give you a hand to complete the job.

@woodser
Copy link
Owner Author

woodser commented Sep 23, 2023

Hey thanks for the offer @everoddandeven. Fortunately this migration is mostly done; all tests are passing in typescript. I just need to finish final cleanup.

@woodser woodser marked this pull request as ready for review October 1, 2023 10:56
@hundehausen
Copy link

🚀 🚀 🚀 🚀

update classes and data models to typescript
add .babelrc, tsconfig.ts, and other ts config
improve stack traces from source maps
replace biginteger.js with BigInt
test and distribute commonjs
update typedocs and other documentation
@woodser woodser merged commit 42d3a4d into woodser:master Oct 1, 2023
woodser added a commit that referenced this pull request Oct 1, 2023
update classes and data models to typescript
add .babelrc, tsconfig.ts, and other ts config
improve stack traces from source maps
replace biginteger.js with BigInt
test and distribute commonjs
update typedocs and other documentation
@hundehausen
Copy link

Congratulations, @woodser !

@woodser
Copy link
Owner Author

woodser commented Oct 1, 2023

Thanks, but still need to get the documentation links working before I can release. :)

@woodser
Copy link
Owner Author

woodser commented Oct 2, 2023

Ok, now it's released and published. :D

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

Successfully merging this pull request may close these issues.

4 participants