Missing package.json declaring module type (ESM/CJS mismatch between tests and lib)

vybe commented

2026-05-19 04:22:24 +00:00

Collaborator

Problem

test/lib.test.js uses ESM import syntax while tailscalectl/lib.js uses CommonJS module.exports. There is no package.json declaring "type": "module". The current setup works only because of Node.js 22’s heuristic module detection and will fail on Node 18 and earlier or under different configuration.

Impact

Tests are not portable across Node versions and rely on an implicit, fragile behavior rather than an explicit module declaration.

## Problem `test/lib.test.js` uses ESM `import` syntax while `tailscalectl/lib.js` uses CommonJS `module.exports`. There is no `package.json` declaring `"type": "module"`. The current setup works only because of Node.js 22’s heuristic module detection and will fail on Node 18 and earlier or under different configuration. ## Impact Tests are not portable across Node versions and rely on an implicit, fragile behavior rather than an explicit module declaration.

vybe commented

2026-05-19 05:47:39 +00:00

Author

Collaborator

Proposed Path Forward — lib.js Cleanup & Test Coverage

Resolves: #20 (for...in without hasOwnProperty), #21 (missing edge-case tests), #22 (redundant duplicate keys in errorMessage), #24 (missing package.json)

Overview

These four issues are all in the JavaScript layer (lib.js and test infrastructure). #20 is a code quality fix, #21 is test coverage, #22 is dead code removal, and #24 is a missing build artifact. They're all small, self-contained changes.

Plan

Step 1 — Fix for...in iteration (fixes #20)

In findActiveExitNode(), replace:

for (const key in peerMap) {

with:

for (const key of Object.keys(peerMap)) {

This avoids prototype chain traversal. Object.keys() only returns own enumerable properties.

Step 2 — Remove duplicate keys (fixes #22)

In errorMessage(), remove the "up" and "down" keys since they map to the same strings as "connect" and "disconnect", and the QML code only ever calls with "connect", "disconnect", "set", or "status". The function becomes:

function errorMessage(cmd) {
    var messages = {
        "connect": "Failed to connect to Tailscale",
        "disconnect": "Failed to disconnect from Tailscale",
        "set": "Failed to set exit node",
        "status": "Failed to read Tailscale status"
    }
    return messages[cmd] || "Tailscale command failed"
}

Update the test cases that call errorMessage("up") and errorMessage("down") to call errorMessage("connect") and errorMessage("disconnect") instead.

Step 3 — Add edge-case tests (fixes #21)

Add test cases for:

parsePeers(null) → []
parsePeers(undefined) → []
parsePeers({}) → []
parsePeers with peer missing TailscaleIPs → ip is ""
parsePeers with peer missing HostName → hostname is the key
findActiveExitNode(null) → ""
findActiveExitNode({}) → ""

These verify the guard clauses and default values in both functions.

Step 4 — Add package.json (fixes #24)

Create package.json at the repo root:

{
  "type": "module",
  "scripts": {
    "test": "node --test test/lib.test.js"
  }
}

This explicitly declares ESM mode, making the test imports portable across Node versions. The "type": "module" field ensures import syntax works without relying on Node 22's heuristic detection.

Success Criteria

findActiveExitNode uses Object.keys() iteration.
errorMessage has no duplicate keys.
Test suite includes edge cases for null/undefined/empty inputs to parsePeers and findActiveExitNode.
package.json exists with "type": "module".
All tests pass: node --test test/lib.test.js exits 0.

AFK Classification

AFK. Pure JavaScript changes with automated test verification. The agent can make all edits, run the tests, and confirm they pass. No HITL needed.

Files Modified

tailscalectl/lib.js — for...in → Object.keys, remove duplicate keys
test/lib.test.js — update existing tests, add edge-case tests
package.json — new file

Also resolves: #20, #21, #22

## Proposed Path Forward — lib.js Cleanup & Test Coverage **Resolves:** #20 (for...in without hasOwnProperty), #21 (missing edge-case tests), #22 (redundant duplicate keys in errorMessage), #24 (missing package.json) --- ### Overview These four issues are all in the JavaScript layer (`lib.js` and test infrastructure). #20 is a code quality fix, #21 is test coverage, #22 is dead code removal, and #24 is a missing build artifact. They're all small, self-contained changes. ### Plan **Step 1 — Fix for...in iteration (fixes #20)** In `findActiveExitNode()`, replace: ```js for (const key in peerMap) { ``` with: ```js for (const key of Object.keys(peerMap)) { ``` This avoids prototype chain traversal. `Object.keys()` only returns own enumerable properties. **Step 2 — Remove duplicate keys (fixes #22)** In `errorMessage()`, remove the `"up"` and `"down"` keys since they map to the same strings as `"connect"` and `"disconnect"`, and the QML code only ever calls with `"connect"`, `"disconnect"`, `"set"`, or `"status"`. The function becomes: ```js function errorMessage(cmd) { var messages = { "connect": "Failed to connect to Tailscale", "disconnect": "Failed to disconnect from Tailscale", "set": "Failed to set exit node", "status": "Failed to read Tailscale status" } return messages[cmd] || "Tailscale command failed" } ``` Update the test cases that call `errorMessage("up")` and `errorMessage("down")` to call `errorMessage("connect")` and `errorMessage("disconnect")` instead. **Step 3 — Add edge-case tests (fixes #21)** Add test cases for: - `parsePeers(null)` → `[]` - `parsePeers(undefined)` → `[]` - `parsePeers({})` → `[]` - `parsePeers` with peer missing `TailscaleIPs` → ip is `""` - `parsePeers` with peer missing `HostName` → hostname is the key - `findActiveExitNode(null)` → `""` - `findActiveExitNode({})` → `""` These verify the guard clauses and default values in both functions. **Step 4 — Add package.json (fixes #24)** Create `package.json` at the repo root: ```json { "type": "module", "scripts": { "test": "node --test test/lib.test.js" } } ``` This explicitly declares ESM mode, making the test imports portable across Node versions. The `"type": "module"` field ensures `import` syntax works without relying on Node 22's heuristic detection. ### Success Criteria 1. `findActiveExitNode` uses `Object.keys()` iteration. 2. `errorMessage` has no duplicate keys. 3. Test suite includes edge cases for null/undefined/empty inputs to `parsePeers` and `findActiveExitNode`. 4. `package.json` exists with `"type": "module"`. 5. All tests pass: `node --test test/lib.test.js` exits 0. ### AFK Classification **AFK.** Pure JavaScript changes with automated test verification. The agent can make all edits, run the tests, and confirm they pass. No HITL needed. ### Files Modified - `tailscalectl/lib.js` — for...in → Object.keys, remove duplicate keys - `test/lib.test.js` — update existing tests, add edge-case tests - `package.json` — new file **Also resolves:** #20, #21, #22