Skip to content

Fixed ipv4 host binding and sockjs proxy exclusion #17

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

Closed
wants to merge 2 commits into from

Conversation

Stieneee
Copy link
Contributor

Hi,

After initial setup of the plugin I ran into some bugs.

The first bug I identified was the fact that the express server was not binding correctly.

The second being the fact that sockjs was being proxied to the express server even though it should not. While I am not certain I believe sockjs is used for communication between the frontend and devServer.

Both can be seen in the image below.

Screenshot from 2019-07-09 15-44-03

Host Binding Fix

Looking at the the server.listen() definition we can see that the default behavior is to bind to ipv6 if enabled. Propagating the host variable to the function call fixed this issue.

Proxy Sockjs Fix

I did not find an easy way to exclude paths in the proxy definition so I opted to create a proxy spec for each route that should be proxied to the express server. This was done by saving the express routes information in parallel with the serverUrl information.

Result

Screenshot from 2019-07-10 11-57-00

Thanks for the plugin!

Stieneee added 2 commits July 10, 2019 11:35
proxy only needed routes to express server to allow sockjs to communicate with devserver
@mathieutu
Copy link
Owner

Hi, thanks for your contribution!
I didn't know for the server.listen function, thanks for that.

I will have specific questions about the proxy part.

Could you please separate that in both PRs for the both fixes/features?

Thanks.

@mathieutu mathieutu self-assigned this Jul 12, 2019
@Stieneee
Copy link
Contributor Author

Stieneee commented Jul 12, 2019

PR has been split.

Fire away with your questions when you can.

@mathieutu
Copy link
Owner

Thanks.

@mathieutu mathieutu closed this Jul 12, 2019
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