feat: Typescript by PuruVJ · Pull Request #23 · beyonk/http

PuruVJ · 2025-05-06T09:48:50Z

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.

leandrohlsilva

LGTM given comments

leandrohlsilva · 2025-05-16T14:23:45Z

+  /** API endpoint */
+  endpoint: string | null;
+  /** HTTP method */
+  method: 'get' | 'GET' | 'post' | 'POST' | 'put' | 'PUT' | 'patch' | 'PATCH' | 'delete' | 'DELETE';


Why not just uppercase?

Just keeping parity with how fetch itself works

Whilst this is a reasonable goal, I don't think we don't need to strive for fetch parity here. I'd go for one or the other - flexibility is the devil!

Haha happy to fix it to uppercase

leandrohlsilva · 2025-05-16T14:24:22Z

+export type RequestConfig = {
+  /** API endpoint */
+  endpoint: string | null;
+  /** HTTP method */


These comments are irrelevant and can be removed.

I would digress, these comments show up in the hover popup in IDEs, so lets leave these here. Can be filled with useful info later on

So these are TSDoc comments?

Yes. Hover wil show it. U can even put code snippets in these

Co-authored-by: Leandro Silva <leandrohlsilva@gmail.com>

PuruVJ · 2025-05-19T11:25:33Z

BTW I forgot to add, to be able to take advantage of this, tsconfig of the consumer projects will have to be changed to not ignore the node_modules. It didn't break anything, just starts including type data from node_modules which is precisely is what we need here

antony

Just adding some comments. I think it's hard to vote with approvals, so I'd suggest a thumbs up or thumbs down as go/no-go

antony · 2025-05-19T13:26:20Z

    "test": "mocha './!(node_modules)/**/**.+(spec).js'",
-    "lint": "eslint lib --ext .js"
+    "lint": "eslint lib --ext .js",
+    "compile": "tsup"


I'm confused at the concept of no build step, but there is a compile step here

No no, it can have a build or no-build. Its what everyone shud decide. I wrote up in the description

Not sure compile is the right word here. I'd suggest build instead

Replaced it with build

antony · 2025-05-19T13:26:50Z

    "node": "18.15.0"
  },
-  "packageManager": "pnpm@8.0.0"
+  "packageManager": "pnpm@9.15.0"


generally this shouldn't be done as part of a PR, but it's a lib so I'm fine with it. Why 9 and not 10?

I just put the one which is installed on my machine 😅

antony · 2025-05-19T13:47:13Z

+  /** API endpoint */
+  endpoint: string | null;
+  /** HTTP method */
+  method: 'get' | 'GET' | 'post' | 'POST' | 'put' | 'PUT' | 'patch' | 'PATCH' | 'delete' | 'DELETE';


Whilst this is a reasonable goal, I don't think we don't need to strive for fetch parity here. I'd go for one or the other - flexibility is the devil!

tomlewis0 · 2025-05-20T13:20:33Z

+   * @returns {Promise<QueryResult>} Response data
+   * @throws {HttpError} If the request fails
+   */
+  async #doQuery(attempt, client, endpoint, options) {


What's with this # syntax? For internal methods I'm more used to seeing _

# is the official private method https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_properties

School day. Thank you!

tomlewis0 · 2025-05-20T13:23:23Z

    "test": "mocha './!(node_modules)/**/**.+(spec).js'",
-    "lint": "eslint lib --ext .js"
+    "lint": "eslint lib --ext .js",
+    "compile": "tsup"


Not sure compile is the right word here. I'd suggest build instead

- Introduced a new ESLint configuration file that extends recommended and mocha rules, setting ECMAScript version to 2025 and source type to module. - Created a pnpm workspace configuration to specify only built dependencies, including 'unrs-resolver'.

pkg-pr-new · 2025-06-18T11:57:53Z

Open in StackBlitz

npm i https://pkg.pr.new/beyonk-group/http/@beyonk/http@23

commit: ca4ef9e

… in package.json

antony · 2025-06-18T14:03:49Z

+        run: pnpm build
+
+      - name: Release
+        run: pnpm dlx pkg-pr-new publish


we have a workflow for this already, it's better that we use it.

I found this: https://github.com/beyonk-group/workflows/blob/main/.github/workflows/canary-libs.yml

Any examples I can check on how to integrate this? Or is it copy-pasta? 😁

https://github.com/beyonk-group/lander/blob/master/.github/workflows/canary-libs.yml

Started using this, now the Action doesn't run 🤔

Ah IG its because workflows is a private repo while this one is public

antony

Left a link to show you how to use the shared workflows

… reference

…workflow

antony · 2025-06-25T10:54:39Z

All looks good, but it occurred to me that as you say - a public repo can't use a private workflow!

Might have to revert to the existing pkg.pr.new build file for this, sorry! :/

…ion spacing in index.js

PuruVJ added 3 commits May 6, 2025 14:44

Push

3c24331

Oops

1c20cae

CJS

74ea2a5

PuruVJ requested a review from tomlewis0 May 6, 2025 09:57

leandrohlsilva approved these changes May 16, 2025

View reviewed changes

PuruVJ and others added 2 commits May 19, 2025 16:54

Update tsup.config.js

dc56e0f

Co-authored-by: Leandro Silva <leandrohlsilva@gmail.com>

Update tsconfig.json

4816b4c

Co-authored-by: Leandro Silva <leandrohlsilva@gmail.com>

antony reviewed May 19, 2025

View reviewed changes

tomlewis0 reviewed May 20, 2025

View reviewed changes

alexdilley approved these changes May 28, 2025

View reviewed changes

PuruVJ added 6 commits June 18, 2025 16:45

dev no-build environment

df3d613

Normalize HTTP method casing in RequestConfig and update default method

5862a0f

Add cr.yml

a20e5c5

eh

e2ffa8d

no compact

4e3d11c

PuruVJ added 6 commits June 18, 2025 17:31

fix: correct exports configuration in package.json

c722542

fix: update ESLint and tsup configurations for improved module handling

4e21c6a

fix: update files configuration in package.json to include lib directory

a24bd44

fix: remove lib directory from files and update exports configuration…

9b36c26

… in package.json

fix: update target in tsup configuration to es2023

b01f89d

fix: implement simpleClone for deep cloning DEFAULT_CONFIG in Api class

c13694b

antony reviewed Jun 18, 2025

View reviewed changes

antony reviewed Jun 23, 2025

View reviewed changes

PuruVJ added 3 commits June 23, 2025 16:19

fix: update GitHub Actions workflow for PRs to use canary-libs job

5e840af

fix: update paths in GitHub Actions workflow to correct lib directory…

7f376b8

… reference

fix: remove paths filter from pull request trigger in GitHub Actions …

eae17cc

…workflow

PuruVJ requested a review from antony June 24, 2025 09:43

antony approved these changes Jun 25, 2025

View reviewed changes

PuruVJ added 4 commits June 25, 2025 20:15

fix: update GitHub Actions workflow to implement continuous releases

6c4e0b6

fix: rename job from test to publish in GitHub Actions workflow

460d057

fix: correct usage of globalIgnores in ESLint config and adjust funct…

90958e7

…ion spacing in index.js

12.1.0

ca4ef9e

PuruVJ merged commit 1ee53b7 into master Jun 26, 2025
3 checks passed

PuruVJ deleted the feature/by-1465 branch June 26, 2025 10:41

Conversation

PuruVJ commented May 6, 2025

Why build step?

Why We Migrated From Mixin Composition to Direct Class Implementation

The Problem

The Solution

Uh oh!

leandrohlsilva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PuruVJ commented May 19, 2025

Uh oh!

antony left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pkg-pr-new Bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antony left a comment

Choose a reason for hiding this comment

Uh oh!

antony commented Jun 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

pkg-pr-new Bot commented Jun 18, 2025 •

edited

Loading