Repository: delvedor/find-my-way
Pull request #333 (2023-08-28 22:28:28)

feat!: parameter object is null object
see https://github.com/fastify/fastify/issues/4357

@eomm
@mcollina
@jsumners

I did not bench

Uzlopak: tests fail because of tap issues...
Uzlopak: Need to benchmark. 
Uzlopak: @climba03003 

It seems that new NullObject is faster than using __proto__ trick

```sh
node benchmark/null-object.js 
new NullObject() x 1,377,691,231 ops/sec ±0.11% (194 runs sampled)
{__proto__: null } x 33,097,458 ops/sec ±0.50% (185 runs sampled)
```

```js
'use strict'

const { NullObject } = require('../lib/null-object')

const Benchmark = require('benchmark')
Benchmark.options.minSamples = 100

const suite = Benchmark.Suite()

suite.add(`new NullObject()`, function () {
  const result = new NullObject()
})
suite.add(`{__proto__: null }`, function () {
  const result = {__proto__: null }
})
suite
  .on('cycle', function (event) {
    console.log(String(event.target))
  })
  .on('complete', function () {
  })
  .run()

```
mcollina: Can you exclude windows v14 from CI?
Uzlopak: @mcollina done.

I would like to write another benchmark for the actual parameter generation. So please dont merge to fast.

I posted my benchmark to discuss if __proto__ is the right way or not.
mcollina: PTAL @ivan-tymoshenko 
ivan-tymoshenko: @Uzlopak Can you post the bechmarks comparison pls?
mcollina: can you merge main?

Patch:
diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml
index 33e3d24..3356004 100644
--- a/.github/workflows/node.js.yml
+++ b/.github/workflows/node.js.yml
@@ -23,6 +23,7 @@ jobs:
           - 18
           - 20
           - 21
+          - 22
         os: 
           - ubuntu-latest
           - windows-latest
@@ -34,6 +35,8 @@ jobs:
             node-version: 14
           - os: macos-latest
             node-version: 16
+          - os: windows-latest
+            node-version: 22 
 
     steps:
     - uses: actions/checkout@v4
diff --git a/index.js b/index.js
index 21297bc..97002bf 100644
--- a/index.js
+++ b/index.js
@@ -115,7 +115,7 @@ Router.prototype.on = function on (method, path, opts, handler, store) {
     assert(path.length === optionalParamMatch.index + optionalParamMatch[0].length, 'Optional Parameter needs to be the last parameter of the path')
 
     const pathFull = path.replace(OPTIONAL_PARAM_REGEXP, '$1$2')
-    const pathOptional = path.replace(OPTIONAL_PARAM_REGEXP, '$2')
+    const pathOptional = path.replace(OPTIONAL_PARAM_REGEXP, '$2') || '/'
 
     this.on(method, pathFull, opts, handler, store)
     this.on(method, pathOptional, opts, handler, store)
diff --git a/lib/handler-storage.js b/lib/handler-storage.js
index 12cd046..d55897b 100644
--- a/lib/handler-storage.js
+++ b/lib/handler-storage.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const { NullObject } = require('./null-object')
 const httpMethodStrategy = require('./strategies/http-method')
 
 class HandlerStorage {
@@ -61,11 +62,20 @@ class HandlerStorage {
   }
 
   _compileCreateParamsObject (params) {
-    const lines = []
+    const fnBody = []
+
+    fnBody.push('const fn = function _createParamsObject (paramsArray) {')
+
+    fnBody.push('const params = new NullObject()')
     for (let i = 0; i < params.length; i++) {
-      lines.push(`'${params[i]}': paramsArray[${i}]`)
+      fnBody.push(`params['${params[i]}'] = paramsArray[${i}]`)
     }
-    return new Function('paramsArray', `return {${lines.join(',')}}`)  // eslint-disable-line
+    fnBody.push('return params')
+    fnBody.push('}')
+
+    fnBody.push('return fn')
+
+    return new Function('NullObject', fnBody.join('\n'))(NullObject)  // eslint-disable-line
   }
 
   _getHandlerMatchingConstraints () {
diff --git a/lib/null-object.js b/lib/null-object.js
new file mode 100644
index 0000000..0740f04
--- /dev/null
+++ b/lib/null-object.js
@@ -0,0 +1,8 @@
+'use strict'
+
+const NullObject = function () {}
+NullObject.prototype = Object.create(null)
+
+module.exports = {
+  NullObject
+}
diff --git a/package.json b/package.json
index b266a60..13f2452 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "find-my-way",
-  "version": "8.1.0",
+  "version": "8.2.0",
   "description": "Crazy fast http radix based router",
   "main": "index.js",
   "types": "index.d.ts",


Test patch:
diff --git a/test/null-object.test.js b/test/null-object.test.js
new file mode 100644
index 0000000..57a9471
--- /dev/null
+++ b/test/null-object.test.js
@@ -0,0 +1,36 @@
+'use strict'
+
+const { test } = require('tap')
+const { NullObject } = require('../lib/null-object')
+
+test('NullObject', t => {
+  t.plan(2)
+  const nullObject = new NullObject()
+  t.ok(nullObject instanceof NullObject)
+  t.ok(typeof nullObject === 'object')
+})
+
+test('has no methods from generic Object class', t => {
+  function getAllPropertyNames (obj) {
+    var props = []
+
+    do {
+      Object.getOwnPropertyNames(obj).forEach(function (prop) {
+        if (props.indexOf(prop) === -1) {
+          props.push(prop)
+        }
+      })
+    } while (obj = Object.getPrototypeOf(obj)) // eslint-disable-line
+
+    return props
+  }
+  const propertyNames = getAllPropertyNames({})
+  t.plan(propertyNames.length + 1)
+
+  const nullObject = new NullObject()
+
+  for (const propertyName of propertyNames) {
+    t.notOk(propertyName in nullObject, propertyName)
+  }
+  t.equal(getAllPropertyNames(nullObject).length, 0)
+})
diff --git a/test/optional-params.test.js b/test/optional-params.test.js
index 2d9c8eb..c1534ba 100644
--- a/test/optional-params.test.js
+++ b/test/optional-params.test.js
@@ -195,3 +195,23 @@ test('deregister a route with optional param', (t) => {
   t.notOk(findMyWay.find('GET', '/a/:param/b'))
   t.notOk(findMyWay.find('GET', '/a/:param/b/:optional'))
 })
+
+test('optional parameter on root', (t) => {
+  t.plan(2)
+  const findMyWay = FindMyWay({
+    defaultRoute: (req, res) => {
+      t.fail('Should not be defaultRoute')
+    }
+  })
+
+  findMyWay.on('GET', '/:optional?', (req, res, params) => {
+    if (params.optional) {
+      t.equal(params.optional, 'foo')
+    } else {
+      t.equal(params.optional, undefined)
+    }
+  })
+
+  findMyWay.lookup({ method: 'GET', url: '/', headers: {} }, null)
+  findMyWay.lookup({ method: 'GET', url: '/foo', headers: {} }, null)
+})