baseprime / grapnel

The smallest JavaScript router with named parameters, HTML5 pushState, and middleware support
http://grapnel.js.org
468 stars 40 forks source link

0.7.x breaks the pattern matching in get method #75

Closed crisperpo closed 4 years ago

crisperpo commented 4 years ago

Hi there,

We have been using Grapnel router in our project, and it works really well for our requirements, so we want to keep using it.

However we have had an issue when upgrading the library from version 0.6.4 to 0.7.2 because the router.get method does not bind the specified path pattern to the given function. It only works as expected for the wildcard '*'.

So the following works as expected and myHandler gets fired for the wildcard '*':

const router = new Grapnel({ pushState: true, root: MY_ROOT })
router.grapnel.get('*', myHandler);

For any other path pattern value different than '*' however the handler is never fired. For example:

const router = new Grapnel({ pushState: true, root: MY_ROOT })
router.grapnel.get('/:learningLanguage/:flowId', myHandler);

After doing some research we found out that the issue happens because of the following:

  1. The add method is called by the get method

  2. The route is created based on the fullPath value:

    let req = route.parse(this.path());
  3. The root is removed from the route:

let req = route.parse(this.path());
  1. The script checks if the route matches the pattern after removing the root:

    if (req.match) {
  2. No matches are found so the function is not bound to the specified path

We found out that the solution to this issue is to use routePath instead of fullPath when creating the route as follows:

let route = new route_1.default(routePath);

We have addressed the issue in the following fork that we are currently using as a dependency in our project:

https://github.com/crisperpo/grapnel

We do not have the permissions to create a pull request from this fork to your repository, or to create a branch, so we have decided to raise this issue. It would be awesome if you could check it out, and help us address this :)

rockaBe commented 4 years ago

Let's get that fixed soon

baseprime commented 4 years ago

@rockaBe @crisperpo There was a problem with the build scripts. They've been reverted to 0.6.x's build system, so please update to the latest version grapnel@0.7.6