EasyPost / easypost-node

EasyPost Shipping API Client Library for Node
https://easypost.com/docs/api
MIT License
139 stars 54 forks source link

Converts project to TypeScript #449

Open ralexmatthews opened 6 months ago

ralexmatthews commented 6 months ago

Description

This converts the project to TypeScript. In order to do this, a handful of kind of major-ish things needed to be done.

Add TypeScript build step

Instead of trying to get Webpack/Babel to compile the TypeScript, I made it so that TypeScript did the first compilation from TS -> JS. Doing it this way was both simpler, and we needed to do a TypeScript compilation anyway to create the TypeDefs. This process includes:

Tests

Because the tests are JS and relied on the JS src code, I just changed the tests to instead use the out dir, since it should be effectively the same. This does mean that in order to run the tests, you need to build first.

Models

Now that we are using TypeScript, I thought the models directory was kind of redundant. Since they were mostly just "structs", classes with no methods or anything, they functioned the same as just a regular object would. And now that we have the types, we get most of the value they were giving anyway. Also this reduces the bundle size non-trivially.

I realize this is a huge change, and if it can't be accepted all at once, that is fine, and I am more than willing to work with you to do this incrementally or whatever

Testing

I ran the tests with my EasyPost API keys and they all passed.

EASYPOST_TEST_API_KEY="EZTK..." EASYPOST_PROD_API_KEY="EZAK..." npm run test

Pull Request Type

Please select the option(s) that are relevant to this PR.

ralexmatthews commented 6 months ago

Hmmm... I am now realizing the way the linting works in this project doesn't allow for typescript support trivially. It looks like it is symlink-ing its eslint configs from the examples repo? If I am reading this right, that falls back to babel/webpack for parsing files, which is not set up here. May need to look deeper into that...

We'll want to determine if this is the route we want to take longterm before fully committing

Is this meaning like if we want to move to TypeScript? Or what route we want to take to move to to TypeScript?

It's also a massive PR, is there an approach we can discuss to transition the project piece by piece or batches at a time?

Hmmm... if you'd rather, I can see if there is a way to maybe allow for JS/TS together in the src dir, and just put up PR after PR converting the files to TypeScript? It'd be a shame to have to make like 15 different versions for implementing TS tho since the work would already be done... but I'm down for whatever.

Justintime50 commented 6 months ago

Is this meaning like if we want to move to TypeScript? Or what route we want to take to move to to TypeScript?

Correct, we didn't have concrete plans to move this to Typescript though it was discussed in the past. For instance, does our set of tooling work with Typescript, are maintaining devs prepared to support Typescript instead of JS, how does it affect the build process longterm, what downstream affects does this have for end-users and how can we pair that with other breaking changes in a single major release, etc. There's a lot to consider with migrating a project from one language to another (superset).

Hmmm... if you'd rather, I can see if there is a way to maybe allow for JS/TS together in the src dir, and just put up PR after PR converting the files to TypeScript? It'd be a shame to have to make like 15 different versions for implementing TS tho since the work would already be done... but I'm down for whatever.

Yeah I think we should discuss options internally as a team. As it stands, 4k and 6k line changes is just way too big to safely review in one go. When we are preparing a new major release of our libs, we typically create a new major version branch and merge in the various feature branches to it over the course of weeks/months while keeping the master branch as the current version. This way we can slowly rollup a bunch of changes while validating it separately from the master branch as we prep the next release. Something like this would fall into that category.

I'm not opposed to swapping to Typescript, I do feel there are a few unanswered questions we should explore and then determine how best to proceed so it's easy for you (can we avoid 15 PRs) but easy for reviewers too (can we batch things into 3 or 4 PRs).

ralexmatthews commented 4 months ago

Hey, so after some talks not in this PR, it looks like it is generally accepted that we will be moving forward with TypeScript? If that is the case, how can I get this diff into an acceptable state to move forward?

Justintime50 commented 4 months ago

@ralexmatthews I'll follow up with you on Slack.