search

tsndr / cloudflare-worker-jwt

A lightweight JWT implementation with ZERO dependencies for Cloudflare Workers.
MIT License
649 stars 51 forks source link

Migration to Vanilla JS with JSDocs #67

Closed vnphanquang closed 6 months ago

vnphanquang commented 7 months ago

Preface

This is a big change (not breaking) and I am sorry if i crossed any line here. I understand both the benefits and arguments against vanilla JS so I will respect whatever eventual decision made by code owner and other maintainers. Discussions are welcome. Thanks.

Changes

The PR is split into multiple commits, each with its own purpose. I recommend reviewing one by one.

  1. Migrating to vanilla JS: will be discussed more below
  2. Mirating jest to vitest. The rationale behind this is that vitest provides better ergonomics in general, especially when working with ESM and Typesript. After previous refactor (1), jest complains about ESM import syntax, while vitest works out of the box. Added also are Vitest UI, and coverage report, run npm run test:ui to try it out.

Context

Following https://github.com/tsndr/cloudflare-worker-jwt/issues/66, the current output of tsc is not compatible with Node's ESM algorithm, which requires explicit .js extension in relative import statements. This PR addresses this issue by migrating the current source code to vanilla JS (with JSDocs), removing a build step completely and allowing explicit .js from source code.

Alternatives

Other ways to get around #66:

  1. Inline utils.ts into index.ts: quick & dirty, but lose code organization
  2. Add a bundling step, using rollup, esbuild, or tsup for example, to squash all source code to a single output file: add to dependency and complexity of the project

About Vanilla JS

The most significant benefit of using vanilla JS in a lib is that there is no build step. In combination with declaration map (setting declarationMap to true in tsconfig), this greatly reduces development friction by letting us inspect and ticker directly with the lib source code from user land.

That being said, this package is targeted to run in cloudflare environment, and "wrangler runs esbuild by default as part of the dev and publish commands, and bundles your Worker project into a single Worker script". So, the aforementioned benefit is certainly there but arguably not that significant.

On the other hand, working with vanilla requires, unsurprisingly, JSDocs. I personally don't mind this given I've published a dozen of libs with vanilla JS, some of quite complex typing. During the refactor, I saw that most things in index.ts have some level of JSDocs already, so all left to do was removing TS-specific syntax and moving some more complex typing to a separate types.ts file for better ergonomics. All in all the development experience won't change that much, but i'll leave that for you to judge.

Change to Publishing Process

The build script has been changed to dts for generating type only. This means source code in src are shipped as is.

A prepublishOnly is added to run tsc, so theoretically npm publish is the only command needed for publication.

tsndr commented 6 months ago

Message from #66:

I really appreciate your effort with the PR (#67), but I've just switched over from js to ts a little while ago and would like to keep it that way. I think a bundler is the best way forward for now, but I'm also always open for feedback and suggestions :)