Skip to content

Conversation

@scott113341
Copy link

This is a very simple fix for #16. Took me a bit to understand the code via the debugger, but this passes all the tests, including the failing test case I added.

For reference, this was the existing test case (passing):

it('should return match to a notFound pattern if provided', () => {
  const {path, value} = switchPath('/home/33/books/10', {
    '/': 123,
    '/authors': 234,
    '/books': {
      '/': 345,
      '/:id': 456
    },
    '*': 'Route not defined'
  });
  expect(path).to.be.equal('/home/33/books/10');
  expect(value).to.be.equal('Route not defined');
});

And the failing one I added, which is identical, but non-nested:

it('should return match to a notFound pattern if provided non-nested configuration', () => {
  const {path, value} = switchPath('/home/33/books/10', {
    '/': 123,
    '/authors': 234,
    '/books': 345,
    '/books/:id': 456,
    '*': 'Route not defined'
  });
  expect(path).to.be.equal('/home/33/books/10');
  expect(value).to.be.equal('Route not defined');
});

Feel free to close if you don't think this is a robust enough solution to the problem. Also, ping @TylorS and @Cmdv because they were working on this before in #17 and #18.

@scott113341
Copy link
Author

Well shoot, I forgot that Travis won't run the first commit with just the failing test case (to prove it's failing). Take my word for it, or:

git clone https://github.com/scott113341/switch-path.git @scott113341-switch-path
cd @scott113341-switch-path
git checkout e439d6f
npm i
npm test

@scott113341
Copy link
Author

I think I need to add some more tests...

@TylorS
Copy link
Collaborator

TylorS commented Dec 3, 2016

LGTM 👍

@janat08
Copy link
Contributor

janat08 commented Aug 31, 2018

Or you can just merge it already. There's a bug it's fixed

@scott113341 scott113341 closed this Nov 4, 2022
@scott113341 scott113341 deleted the not-found-fix branch November 4, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants