Ponytail on expressjs/cors — 1 review · 1 audit · 1 ledger · 1 minimal diff

/ponytail full --tee ./out/ upstream · DietrichGebert/ponytail sample repo · expressjs/cors

Review Findings

delete-list, pre-push

Audit Findings

whole-repo sweep

Debt Notes

ponytail: harvested

Diff Landed

before → after

/ponytail-review ./out/ponytail-review.md

Delete-list verdict

6 findings — 3 delete · 2 replace-with-stdlib · 1 collapse

Every finding has a real file:line anchor. No new abstraction proposed.

01 stdlib lib/index.js:5

object-assign → native Object.assign

Native Object.assign has shipped since Node v4. This shim is kept alive only by engines.node: ">= 0.10". Drop shim + drop dep.

Replace

02 delete lib/index.js:15-17

isString() — inline the check

s instanceof String matches only boxed strings — a value nobody writes on purpose. Inline typeof x === 'string' at both callers.

Delete

03 delete lib/index.js:1,238

IIFE wrapper on a CommonJS module

Pre-CommonJS browser-globals pattern. Node modules already scope; every reader has to un-wrap it before reading the module body. Delete the wrapper, keep one top-level 'use strict'.

Delete

04 collapse lib/index.js:117-142

Double-guard in configureExposedHeaders / configureMaxAge

After if (!headers) return null;, the follow-up if (headers && …) is dead. Same shape twice. Drop the redundant conjunct — do not extract a shared helper.

Collapse

05 stdlib lib/index.js:144-157

applyHeaders recursion → Array#flat

Recursion exists only because headers.push([{…}]) nests arrays. Array.prototype.flat(Infinity) collapses them; the branch on Array.isArray disappears.

Replace

06 delete lib/index.js:31-33

Trailing else in isOriginAllowed is unreachable

After Array / string / RegExp branches, the last else returns !!allowedOrigin — but null / undefined / boolean are already handled upstream at lib/index.js:41. Delete.

Delete

/ponytail-audit ./out/ponytail-audit.md · ranked by impact ÷ effort

Files scanned

Drop-in

Small refactor

Needs test

Top-of-list — reuse-first wins

A Drop object-assign; use native Object.assign lib/index.js:5,208 · package.json:20

Native shipped in Node v4 (2015). Dep survives only because engines.node: ">= 0.10" at package.json:35. Drop 0.10 → delete shim.

ImpactHigh EffortDrop-in

B Delete the IIFE wrapper on lib/index.js lib/index.js:1-4,238

2010-era pattern for browser globals. Same shape repeats one level deeper in test/test.js:6 — a nested IIFE inside a module that already sets 'use strict' on line 1.

ImpactMedium EffortDrop-in

C Delete isString() — inline typeof lib/index.js:15-17

Two callsites, primitive check. −3 LoC, one less "why does this helper exist" moment.

ImpactMedium EffortDrop-in

Mid-of-list — mild archaeology

D util.inherits → class FakeResponse extends EventEmitter test/test.js:1-4,708-737

Class syntax has been in Node since v4; util.inherits is a compat shim. Test-only code — no user impact.

ImpactMedium EffortSmall refactor

E .eslintrc.yml is on ESLint 7 — delete or upgrade .eslintrc.yml · test/.eslintrc.yml · package.json:25,40

Frozen linter that predates modern Node features. The lint job in CI is either silent-passing or noisy-failing. Pick.

ImpactMedium EffortSmall refactor

F Collapse duplicated double-guard shapes lib/index.js:117-131,133-142

if (headers) return null; if (headers && …) — twice. No new helper. Just clean the branches.

ImpactLow EffortDrop-in

G Drop after callback-fanout dep — Promise.all covers it test/test.js:9 · dozens of callsites

after(1, done) is a no-op wrapper; after(n, done) is Promise.all(…).then(done). Removes a 2013-era dep that predates promises.

ImpactMedium EffortSmall refactor

Bottom-of-list — needs a ponytail: note

H Reflect-origin returns false header value; unify with skip-check lib/index.js:60-62,152

Correct today, but ugly downstream. Returning null instead of false for a rejected origin aligns with applyHeaders's truthy filter. Needs a characterization test before touching.

ImpactLow EffortNeeds test

/ponytail-debt harvest

8 ponytail: notes across 4 files

3 delete · 3 stdlib · 1 dedupe · 1 defer — all still live

#1stdliblib/index.js:5

ponytail: object-assign predates native Object.assign; live shim till node ≥18 lands

→ drop shim + drop dep once engines.node bumps

#2deletelib/index.js:16

ponytail: instanceof String branch is dead — no one boxes strings on purpose

→ inline typeof x === 'string' at both callers

#3deletelib/index.js:31

ponytail: trailing else branch never fires; kept as belt-and-suspenders

→ delete lines 31-33; guarded upstream at line 41

#4dedupelib/index.js:148

ponytail: recursion is here only because push([{…}]) nests arrays — flat() would win

→ concat + flat(Infinity); delete recursive branch

#5stdlibtest/test.js:3

ponytail: EventEmitter + util.inherits — class extends does this since node 4

→ rewrite FakeResponse as class; delete require('util')

#6deletetest/test.js:6

ponytail: nested IIFE inside a module already using 'use strict' at line 1

→ unindent, drop wrapper lines; same shape in lib/index.js:1

#7defer.eslintrc.yml

ponytail: pinned to eslint 7 (2020); either delete or upgrade to flat config

→ decide: rely on GH Actions' linter, or migrate to eslint.config.js

#8stdlibtest/test.js:9

ponytail: 'after' predates promises — Promise.all covers all fanout callsites

→ replace after(n, done) with Promise.all([…]).then(done)

One finding, landed

Drop object-assign; use native + delete isString() + delete IIFE wrapper

A + B + C from the audit, in a single reviewable diff

./out/before-after.diff · minimal, reversible

lib/index.js −16 / +7

@@ -1,17 +1,7 @@
-(function () {
+'use strict';
 
-  'use strict';
-
-  var assign = require('object-assign');
-  var vary = require('vary');
+var vary = require('vary');
 
-  function isString(s) {
-    return typeof s === 'string' || s instanceof String;
-  }
@@ -27,7 +17,7 @@ function isOriginAllowed(origin, allowedOrigin) {
     return false;
-  } else if (isString(allowedOrigin)) {
+  } else if (typeof allowedOrigin === 'string') {
     return origin === allowedOrigin;
@@ -205,7 +195,7 @@ function middlewareWrapper(o) {
       } else {
-        var corsOptions = assign({}, defaults, options);
+        var corsOptions = Object.assign({}, defaults, options);
         var originCallback = null;
@@ -235,5 +225,3 @@ function middlewareWrapper(o) {
 module.exports = middlewareWrapper;
-
-}());
            

package.json −2 / +1

@@ -17,7 +17,6 @@
   "dependencies": {
-    "object-assign": "^4",
     "vary": "^1"
   },
@@ -32,6 +31,6 @@
   "engines": {
-    "node": ">= 0.10"
+    "node": ">= 18"
   },
            