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

Typescript support (entry level only) #49

Merged
merged 16 commits into from
Aug 30, 2024
Merged

Typescript support (entry level only) #49

merged 16 commits into from
Aug 30, 2024

Conversation

alienkarma
Copy link
Contributor

@alienkarma alienkarma commented Aug 23, 2024

Attempt to fix issue #41 Enhance Public AgenticJS API TypeScript

  • Added types for relevant entry API
  • Missing type for unknown context (WIP)
  • Added "test:types" script for testing typescript validation
  • Added package type: module

- Added types for relevant entry API
- Missing type for unknown context
- Added "test:types" script for testing typescript validation
- Added package type: module
types/index.d.ts Outdated Show resolved Hide resolved
types/index.d.ts Outdated Show resolved Hide resolved
@darielnoel
Copy link
Contributor

Hi @alienkarma,

Thank you so much for your contributions! Adding TypeScript to AgenticJS is a significant enhancement that will undoubtedly benefit many developers.

I’ve reviewed your pull request and have a few suggestions. Your expertise is truly valuable to us, and your efforts are greatly appreciated. Please know that these suggestions are meant to build on the fantastic foundation you’ve already laid, without any pressure.

  1. Link to Issue & Description in PR: It would be great if you could include a link to the related issue and a brief description in your PR to provide context for other collaborators.

  2. JSDoc Annotations: Do you think it would be beneficial to add JSDoc annotations to the type definitions? I’m under the impression that doing so could enhance IDE autocompletion features for TypeScript users, making the library more intuitive to use. What's your take on this? Would this addition be helpful in your view? If so you can get it from here(https://docs.agenticjs.com/category/core-concepts)

  3. TypeScript Example Project: When you have a moment, could you create a small TypeScript example in the Playground? Including a README with usage examples would be wonderful, and we’d love to showcase this in our main documentation later on.

Thank you again for your hard work and dedication. I’m here to support you in any way I can.

@alienkarma
Copy link
Contributor Author

Nice.
Thank you for the feedback.
I will implement the necessary changes and update once done.

@alienkarma
Copy link
Contributor Author

Hi @darielnoel
I added the JSDocs and also created a typescript version of the nodejs playground example.
The type seems to be working (fixed a few type issues because of this as well).
I will work on a proper typescript react example later on.

To add the types it has to be declared as a separate package @types/agenticjs, assuming the module name is agenticjs.
I am attaching the node_modules directory for more clarity. (The index.js and index.js.map are module.esm.js and module.esm.js.map files respectively)

image

I am not sure yet how to produce a proper distributable output yet using rollup. (except as an external type package)
Anyways, let me know if you have any more feedback and/or changes.

@darielnoel
Copy link
Contributor

Hi @alienkarma,

Thank you for the quick updates and for adding the JSDocs along with the Node.js TypeScript example. 🤗

Could you provide the steps required to set up and run the Node.js TypeScript example locally?
I want to ensure that I follow your setup correctly to fully experience and test the new capabilities you've integrated.

(Maybe adding a Readme.md with the steps is a good idea?)

P.S: I know running such examples can be complex without the module being available on npm. However, I'm confident we can figure out an effective workaround together.

Thanks again for your hard work and dedication!

@alienkarma
Copy link
Contributor Author

alienkarma commented Aug 27, 2024

I will work on making a build setup script going (for both dev and production purpose).
Using this then we can make setting up and testing dev easier, and also publish for production purpose as well.
Will also work on adding the detailed playground example as you requested.
Will update once done.

@darielnoel
Copy link
Contributor

Thank you @alienkarma, I'm sorry for the back and forth. Just trying to make sure that every feature we add has a solid foundation. Here's some of the philosophy we are trying to pursue.

https://resend.com/philosophy

Thanks for your patience and understanding 🤗

@pelikhan
Copy link

Very much looking for TypeScript friendlyness; it really helps diving into a new codebase.

- Added typescript package builder
- Updated node-ts playground example
@alienkarma
Copy link
Contributor Author

Hi @darielnoel
I added a typed package builder that we can import and use as a package for testing (and maybe distribution).
I have also updated the nodejs-ts playground example using the built package and also updated the readme for it as well.

As always, let me know if any changes/feedback. (I really appreciate it 😄)

- Replaced dts-generator with rollup-dts plugin.
- Used npm link to link packages locally for playground testing
- Simplified bundle outputs and scripts
- Modified package json exports to reflect the same
- Fixed unnecessary typing references
@darielnoel darielnoel merged commit ed141b2 into kaiban-ai:main Aug 30, 2024
1 check passed
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.

3 participants