Aligned rpc with tailscale login flow#912
Conversation
f8ddaa3 to
01f7596
Compare
chore: added stricter types fix: rpc manifest
e9cb130 to
5eda0b8
Compare
There was a problem hiding this comment.
I see you have created a client handler and also created the appropriate test for it as well which is very nice. As I am not too used to the Polykey codebase I cannot pick up any design patterns that you might be missing, but generally the code looks fine.
I think the tsconfig module option should be changed back to ESNext as Polykey is to be ESM only. But best to confirm with @tegefaulkes , besides that everything looks good to merge!
|
This PR looks good to go, so I'm merging it now. |
Context and AssumptionsAssumptions:
1. Definitions and Intent
|
| Aspect | "ESNext" | "NodeNext" |
|---|---|---|
| ESM syntax | Yes | Yes |
| Node ESM resolver rules | No | Yes |
| Honors package exports | No | Yes |
| Handles .mts/.cts | No | Yes |
| Extension rewriting | No | Yes |
| Output for Node.js | Brittle | Seamless |
| Use-case | Browser/Spec-only ESM | Node.js-targeted ESM (recommended) |
Conclusion
For any modern Node.js ESM-only project, always use "module": "NodeNext" in your tsconfig.json.
It ensures complete compatibility with Node’s ESM loader, file extension handling, package boundary rules, and avoids almost all TypeScript–Node ESM interop pain.
"module": "ESNext" is a low-level, spec-only mode—not recommended for practical Node.js development in 2025.
If you have specific edge-case requirements (e.g., multi-target builds, browser bundling, or want to debate browser vs Node ESM interop), clarify and I can break down further.
|
So @aryanjassal is right for a node focused project it should be |
Description
Issues Fixed
polykey auth logincommand Polykey-CLI#408 (REF ENG-620)Tasks
Final checklist