fridays / next-routes

Universal dynamic routes for Next.js
MIT License
2.47k stars 230 forks source link

/:slug matches on /__webpack_hmr #6

Closed sedubois closed 7 years ago

sedubois commented 7 years ago

With 1.0.15:

routes.js:

const nextRoutes = require('next-routes');
module.exports = nextRoutes()
  .add('/')
  .add('about')
  .add('discover')
  .add('retreat', '/retreat/:id')
  .add('track', '/track/:id')
  .add('profile', '/:slug');

This code:

const { route, params } = routes.match(req.url);
console.log('Found next-routes: ', route, params);

(among other things) returns the log below:

``` Found next-routes: Route { name: 'profile', pattern: '/:slug', page: '/profile', keys: [ { name: 'slug', prefix: '/', delimiter: '/', optional: false, repeat: false, partial: false, asterisk: false, pattern: '[^\\/]+?' } ], regex: { /^\/((?:[^\/]+?))(?:\/(?=$))?$/i keys: [ [Object] ] }, getAs: [Function] } { slug: '__webpack_hmr' } ```

/:slug matches on __webpack_hmr, which it shouldn't.

Also not sure if it does or not, but should also ensure it won't match on /favicon.ico either.

fridays commented 7 years ago

.match() is an internal function that matches a string against all registered patterns, in the case of a catch-all route this is expected behavior for that function.

If you use routes.getRequestHandler(app) like documented in the readme, that path won't match. It first checks against routes.isNextPath() and skips if positive. Please check how it's implemented here. You can use it in a similar way for the caching example.