Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give proxy precedence over history api fallback #16

Merged
merged 3 commits into from
Mar 18, 2017

Conversation

chooty
Copy link
Contributor

@chooty chooty commented Mar 8, 2017

Currently the middleware used for the --fallback option is executed before the --proxy middleware. This causes all requests to return index.html, even the ones to the path specified with --proxy-route, in effect making it so --proxy has no effect at all in combination with --fallback.
This change switches the middlewares around so the proxy middleware will proxy any requests to its specified path.

@piotrwitek
Copy link
Owner

piotrwitek commented Mar 9, 2017

@chooty Hi, thanks for contribution, I planned to add some superagent tests to confirm this functionality is working as I'm not using it in my projects with this dev server.

So please let me add some tests first to confirm that it is working and then I gonna merge this PR.

Thanks!

@piotrwitek piotrwitek self-assigned this Mar 9, 2017
@piotrwitek piotrwitek self-requested a review March 9, 2017 08:41
@piotrwitek piotrwitek added the bug label Mar 9, 2017
@piotrwitek piotrwitek added this to the v1.0.0 milestone Mar 9, 2017
@piotrwitek piotrwitek removed the bug label Mar 18, 2017
@piotrwitek
Copy link
Owner

@chooty I have fixed some issues and added tests including your case, it looks like your issue is already fixed.
Can you please update to newest release (v1.0.0-rc5) and confirm it is fixed?

@chooty
Copy link
Contributor Author

chooty commented Mar 18, 2017

@piotrwitek It works so long as the request to the proxied route doesn't have an accept-header that the fallback middleware will pick up.
I would still expect it to proxy the request even if the client sends accept: */*, which it doesn't now from what I can see. It's not a big issue however since it's easy to work around on the client.

@piotrwitek
Copy link
Owner

@chooty thanks for elaborating, so I understand that accept header is having impact here, I would like to fix it, can you help me with improving my tests cases?
Here is the test case: https://github.com/piotrwitek/jspm-hmr/blob/master/test/jspm-hmr-server.spec.ts

Could you give me hints to have a breaking test I could test is catching your specific scenario?

Thanks a lot!

@chooty
Copy link
Contributor Author

chooty commented Mar 18, 2017

@piotrwitek I've merged 1.0.0-rc5 and updated the test for the combination of fallback and proxy. Please have a look.

@piotrwitek
Copy link
Owner

@chooty thanks a lot for helping with this edge case 👍
looks good to me

@piotrwitek piotrwitek merged commit ffe28e5 into piotrwitek:master Mar 18, 2017
@piotrwitek
Copy link
Owner

@chooty gonna push a new release with this change later 'rc6'

@piotrwitek piotrwitek modified the milestones: v1.0.0-rc6, v1.0.0 Mar 18, 2017
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.

2 participants