pillarjs / path-to-regexp

Turn a path string such as `/user/:name` into a regular expression
MIT License
8.18k stars 382 forks source link

Regression > 0.1.7 #271

Closed frank-dspeed closed 2 years ago

frank-dspeed commented 2 years ago

i copy the details from my issue that i got after upgrading from 0.1.7

// packages/express/lib/router/layer.js
require('path-to-regexp').pathToRegexp

// packages/express/examples/downloads/index.js
L22: app.get('/files/:file(*)', function(req, res, next){
0.1.7: path:  /files/:file(*) regexp:  /^\/files\/(?:((.*)))\/?$/i
6.2.0: SyntaxError: Invalid regular expression: /^\/files(?:\/(*))[\/#\?]?$/: Nothing to repeat

// packages/express/test/Router.js
L348-363:  router.handle({ url: '*', method: 'GET' }, { end: cb }, no)
0.1.7:  path:  * regexp:  /^(.*)\/?$/i
6.2.0: TypeError: Unexpected MODIFIER at 0, expected END

Details First Error

SyntaxError: Invalid regular expression: /^\/files(?:\/(*))[\/#\?]?$/: Nothing to repeat
    at new RegExp (<anonymous>)
    at tokensToRegexp (express/node_modules/path-to-regexp/dist/index.js:390:12)
    at stringToRegexp (express/node_modules/path-to-regexp/dist/index.js:331:12)
    at pathToRegexp (express/node_modules/path-to-regexp/dist/index.js:405:12)
    at new Layer (express/lib/router/layer.js:47:19)
    at Function.route (express/lib/router/index.js:500:15)
    at Function.app.<computed> [as get] (express/lib/application.js:481:30)
    at Object.<anonymous> (express/examples/downloads/index.js:22:5)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (express/test/acceptance/downloads.js:2:11)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:190:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:195:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:331:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at async formattedImport (express/node_modules/mocha/lib/nodejs/esm-utils.js:7:14)
    at async Object.exports.requireOrImport (express/node_modules/mocha/lib/nodejs/esm-utils.js:48:32)
    at async Object.exports.loadFilesAsync (express/node_modules/mocha/lib/nodejs/esm-utils.js:103:20)
    at async singleRun (express/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (express/node_modules/mocha/lib/cli/run.js:374:5)
{
  path: '/files/:file(*)',
  opts: { sensitive: false, strict: false, end: true },
  keys: [
    {
      name: 'file',
      prefix: '/',
      suffix: '',
      pattern: '*',
      modifier: ''
    }
  ]
}

Details Secund Error

TypeError: Unexpected MODIFIER at 0, expected END
    at mustConsume (express/node_modules/path-to-regexp/dist/index.js:114:15)
    at parse (express/node_modules/path-to-regexp/dist/index.js:173:9)
    at stringToRegexp (express/node_modules/path-to-regexp/dist/index.js:331:27)
    at pathToRegexp (express/node_modules/path-to-regexp/dist/index.js:405:12)
    at new Layer (express/lib/router/layer.js:47:19)
    at Function.route (express/lib/router/index.js:500:15)
    at Function.proto.<computed> [as all] (express/lib/router/index.js:515:22)
    at Context.<anonymous> (express/test/Router.js:356:14)
    at callFnAsync (express/node_modules/mocha/lib/runnable.js:394:21)
    at Test.Runnable.run (express/node_modules/mocha/lib/runnable.js:338:7)
    at Runner.runTest (express/node_modules/mocha/lib/runner.js:678:10)
    at express/node_modules/mocha/lib/runner.js:801:12
    at next (express/node_modules/mocha/lib/runner.js:593:14)
    at express/node_modules/mocha/lib/runner.js:603:7
    at next (express/node_modules/mocha/lib/runner.js:486:14)
    at Immediate._onImmediate (express/node_modules/mocha/lib/runner.js:571:5)
    at processImmediate (node:internal/timers:464:21)
{
  path: '*',
  opts: { sensitive: undefined, strict: undefined, end: true },
  keys: []
}

i hope some one in here can tell me the migration path i think already the first issue is solved when i replace

/:file(*) // with /:file(.*) or /:file*

but i am not sure about the secund case i need clarification the input parameters and expected and current output are there

my first observation for the first issue is also that it produces the same regex but missing the i at the end i found

but i am still not sure how to fix that i can rewrite the one failing test and i can correct the example and docs to not use only the * but i need clear replacements i am not sure if i got everything right

blakeembrey commented 2 years ago

This not a regression or a bug, the documentation is in the README. If you want the literal translation between old and new formats it's /files/:file(.*) and (.*) (* basically means .* in 0.1.x), feel free to open a PR to add them to the compatibility docs if you think it'd help others!

frank-dspeed commented 2 years ago

@blakeembrey sorry that i ping you again but i tested it and i still get

pathToRegexp get called with {
  path: '.*',
  opts: { sensitive: undefined, strict: undefined, end: true },
  keys: []
}

TypeError: Unexpected MODIFIER at 1, expected END

maybe i need to explicit do /(.*) ?

frank-dspeed commented 2 years ago

Ok i got it all works nice thanks a lot for the good documentation do you know any reason for express to not take the new syntax?

blakeembrey commented 2 years ago

for express to not take the new syntax?

The current express router is still based on the old version, when it upgrades it'll get the new syntax (in express 5).