Skip to content

feat: Typescript #23

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat: Typescript #23

wants to merge 3 commits into from

Conversation

PuruVJ
Copy link

@PuruVJ PuruVJ commented May 6, 2025

CleanShot 2025-05-06 at 15 00 21

A few things about this PR:

  • Keeps the codebase in plain JS, and adding types via JSDoc
  • Adds tsup for bundling(More on that later)
  • Simplifies the codebase into a single file(More on that later)
  • Moved all error handlers to the Api class(Ditto)

Why build step?

I wanna start this sentence with: We do not need a build step. The files are in .js, and will work in runtime. If you import a .js file exposed from a package, into a .js file in a (say sveltekit) app, the typescript will work well there.

The build step however, is there for two possible future cases:

  • We wanna rewrite the package to TypeScript
  • We wanna use this .js package in a .ts package. TypeScript files are strict, and do not infer the types from .js coming from an external package. It strictly demands a .d.ts file attached there, which is where the build step comes in.

So, we actually do not need the build step in the present at all, but if in any of the above cases, a build step generating .d.ts files is must.

Why We Migrated From Mixin Composition to Direct Class Implementation

Our HTTP API client previously used the mixin composition pattern, which created several challenges when integrating TypeScript types.

The Problem

The previous approach:

  • Split error handlers across multiple files
  • Used a complex composition pattern (compose(MixinA, MixinB, ...)(BaseClass))
  • Created a convoluted inheritance chain hard for TypeScript to track
  • Caused bundling issues with tsup/esbuild (like the line 229 error)

TypeScript struggled with this pattern because:

  • It couldn't properly track types through the composition chain
  • this typing became complex across multiple inheritance layers
  • The interplay between JSDoc annotations and dynamic composition confused the bundler, causing it to mysteriously error while generating d.ts file.

The Solution

I consolidated everything into a direct class implementation:

  • Single class with all error handler methods directly defined
  • No complex composition or inheritance chains
  • Grouped related functionality together

This approach works much better with TypeScript because:

  • It follows standard class patterns TypeScript excels at typing
  • Properties and methods can be directly typed without complex inference
  • IDE tooling provides better autocomplete and type hints
  • Bundlers like tsup can process the code more reliably.
  • Slightly smaller bundler size 😁

The runtime behavior remains identical, but we get better developer experience, easier maintenance, and reliable bundling.

@PuruVJ PuruVJ requested a review from tomlewis0 May 6, 2025 09:57
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.

1 participant