Adapt Python SDK to Typescript (E2B-30)

[jzanecook]

Thanks @jzanecook and congratulation for being our first contributor! Ran quick test and seems to be working. Good job!

Thanks!

Is there a reason to prefer this over OpenAPI generator? Did you try this generator?

No, I have not. Should I?

@ValentaTomas correct me, if I'm wrong. The reason for static method is to have one global Agent wrapper and you can then conveniently import it from different files in the codebase without the need to pass it as an argument. At the moment it isn't that important, because there isn't any functionality why would you need importing the Agent yet.

I made the agent use static methods, but ran into issues when both were static, I referenced this article https://stackoverflow.com/a/64733963 and did it that way.

• Let's name the package agent-protocol. I have the name reserved.

Done.

• Can you add script for generating the boilerplate (models, routes, etc.)

I didn't use a script to generate it, I guess that's part of the OpenAPI generators?

• What is the purpose of json-schema-to-typescript in Python SDK?

This was with the pydantic2ts package and should be removed.

• When running npm run fix I am getting this errors

This rule requires the `strictNullChecks` compiler option to be turned on to function correctly  @typescript-eslint/strict-boolean-expressions

Fixed, I added this compiler option.

• You have prettier among scripts, but it isn't among dev dependencies

I had been using the installed prettier, not the package prettier, fixed now.

• Remove jzanecook-e2b-sdk-test from dependecies

Whoopsies.

• When trying to build I am getting this

Entry module "src/index.ts" is using named and default exports together. Consumers of your bundle will have to use `agentSdk["default"]` to access the default export, which may not be what you want. Use `output.exports: "named"` to disable this warning

This was a lot more effort than I was expecting lol, but at the end what I had to do was build CJS and ESM separately, reference the build script. I also ran into issues with the typings going into dist/src and erroring, so I added a little extra to the build script to make that go away.

• It would be nice to have linted examples/ as well

Added the examples to the linting process in tsconfig, also removed them from the build.

[jakubno]

Is there a reason to prefer this over OpenAPI generator? Did you try this generator?

No, I have not. Should I?

I would prefer it. This way you would depend on the python package. If any change happens in the spec, you will have to change adjust python code and after that you can adjust js code.

@ValentaTomas correct me, if I'm wrong. The reason for static method is to have one global Agent wrapper and you can then conveniently import it from different files in the codebase without the need to pass it as an argument. At the moment it isn't that important, because there isn't any functionality why would you need importing the Agent yet.

I made the agent use static methods, but ran into issues when both were static, I referenced this article https://stackoverflow.com/a/64733963 and did it that way.

I added some code suggestions to copy the python behavior.

• Can you add script for generating the boilerplate (models, routes, etc.)

I didn't use a script to generate it, I guess that's part of the OpenAPI generators?

Something like this line. When we change the spec, you will just need to run the command to regenerate the code and maybe do some small adjustments. But it should make it much more maintainable.

• When trying to build I am getting this

Entry module "src/index.ts" is using named and default exports together. Consumers of your bundle will have to use `agentSdk["default"]` to access the default export, which may not be what you want. Use `output.exports: "named"` to disable this warning

This was a lot more effort than I was expecting lol, but at the end what I had to do was build CJS and ESM separately, reference the build script. I also ran into issues with the typings going into dist/src and erroring, so I added a little extra to the build script to make that go away.

I checked microbundle and it doesn't support default and named exports in one file. So I would prefer not mixing or using tsup. This looks too complicated and probably it's bypassing their design decision to not support it.

[jzanecook]

I checked microbundle and it doesn't support default and named exports in one file. So I would prefer not mixing or using tsup. This looks too complicated and probably it's bypassing their design decision to not support it.

That's correct, it was from their github issues I found the resolution with the separated build. You're right though, it's too complicated. I wasn't able to get the tsup build to work at all, though.

[jzanecook]

Is there a reason to prefer this over OpenAPI generator? Did you try this generator?

No, I have not. Should I?

I would prefer it. This way you would depend on the python package. If any change happens in the spec, you will have to change adjust python code and after that you can adjust js code.

Well that's a shame, I was going in the wrong direction the whole time...

I made the agent use static methods, but ran into issues when both were static, I referenced this article https://stackoverflow.com/a/64733963 and did it that way.

I added some code suggestions to copy the python behavior.

Code suggestions?

Something like this line. When we change the spec, you will just need to run the command to regenerate the code and maybe do some small adjustments. But it should make it much more maintainable.

Okay, I'll take a look and see what I can come up with.

[jzanecook]

Can we separate these comments out into individual issues so that we can isolate them and clarify the requirements a little easier?

[jakubno]

Sorry I didn't submit the suggestions 🙈

[jakubno]

Can we separate these comments out into individual issues so that we can isolate them and clarify the requirements a little easier?

Sure, let's get this done. I'll create an issue for generating the types and endpoints. This works and that's the most important thing! 🎉

[jakubno]

So just address the suggestions, rebase on master and let's get it out 💪🏻

[jzanecook]

Pretty sure I rebased properly here, I also:

• Made sure that the build with tsup (which I'm glad worked) also built the types so that if I ran the minimal example with ts-node it wouldn't yell at me.
• Removed index.cjs.ts since it's no longer needed.
• Added the proper typing to the static function in the Agent class so that eslint wouldn't yell at me.
• Successfully verified that nothing is yelling at me! 🎉

PS: It should be noted that eslint and prettier conflict. I've been pushing after prettier, but if you run eslint without prettier it will cause changes in all of the files. We may consider putting the eslint check and the prettier fix into a singular command, or tweaking the eslint to match prettier.

[jakubno]

Just last few nit picks and we'll merge it 🎉

[jzanecook]

Alright @jakubno I think we're 💯

[jakubno]

Thanks @jzanecook for your contribution!