Uses relative paths for urls

[se7entyse7en]

Closes #286 as it makes gitbase-web work both directly and behind a proxy with path other than /.

[se7entyse7en]

Analogously to bblfsh/web#185 I haven't tested this by running gitbase-web behind a Kubernetes proxy as written in issue #286, but for simplicity I've used nginx (I suppose that it should be the same) with the following configuration:

server {
    listen       8081;
    server_name  localhost;

    location /gitbase/ {
        proxy_pass http://127.0.0.1:8080/;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $remote_addr;
        proxy_set_header Host $host;
    }

    location /fonts {
        proxy_pass http://127.0.0.1:8080/fonts;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $remote_addr;
        proxy_set_header Host $host;
    }

}

where gitbase-web was running on port 8080.

With this setup gitbase-web works correctly both by using it directly through localhost:8080 and by using it behind nginx through localhost:8081/gitbase.

I wasn't able to make it work without the routing rule in nginx for the /fonts path, as those are referenced inside the *.less file.

[carlosms]

This part is tricky, so I would expand more this comment. Something like:

If serverUrl is unset, this replaces the leading / in order to work behind proxies with a path other than /.

[carlosms]

Let's apply this comment (or a better one if you see a way to explain it more clearly)

[se7entyse7en]

Oh yes sorry, complete forgot about that!

[carlosms]

The css path problem might be caused by the fact that we are using react-app-rewire-less, and the new "homepage" setting in package.json does not apply. I would not close #286 until we find a solution, but if it's not obvious how to proceed I think we could merge this PR, and then fix it separately.

[carlosms]

I've done a quick test and the fonts use the right path if we move the public/fonts dir to src/fonts, and use relative paths ./fonts/ in fonts.less.

[se7entyse7en]

@carlosms thanks a lot for testing it! Is it ok then to move the fonts and close the issues with this fix?

[carlosms]

Yes, let's move the fonts also in this PR.

[carlosms]

BTW thank you for providing the configuration needed for testing @se7entyse7en. Here is how I'm testing it, with some more details to be ready to c&p.
For the sake of keeping this in the repo history, in case somebody needs to review this or we want to automate some tests in the future.

File nginx.conf, contains the defaults + @se7entyse7en's conf:

user  nginx;
worker_processes  1;

error_log  /var/log/nginx/error.log warn;
pid        /var/run/nginx.pid;


events {
    worker_connections  1024;
}


http {
    include       /etc/nginx/mime.types;
    default_type  application/octet-stream;

    log_format  main  '$remote_addr - $remote_user [$time_local] "$request" '
                      '$status $body_bytes_sent "$http_referer" '
                      '"$http_user_agent" "$http_x_forwarded_for"';

    access_log  /var/log/nginx/access.log  main;

    sendfile        on;
    #tcp_nopush     on;

    keepalive_timeout  65;

    #gzip  on;

    include /etc/nginx/conf.d/*.conf;

    server {
        listen       8081;
        server_name  localhost;

        location /gitbase/ {
            proxy_pass http://127.0.0.1:8080/;
            proxy_set_header X-Real-IP $remote_addr;
            proxy_set_header X-Forwarded-For $remote_addr;
            proxy_set_header Host $host;
        }
    }
}

Run nginx docker container:

docker run -v $PWD/nginx.conf:/etc/nginx/nginx.conf:ro --network host nginx

Now http://localhost:8081/gitbase/ is a gateway to http://localhost:8080, so you can run ./build/bin/gitbase-web serve with the default settings.

[se7entyse7en]

@carlosms after moving the fonts and changed fonts.less:

mkdir -p build
yarn --cwd ./frontend build
yarn run v1.12.3
$ react-app-rewired build
Creating an optimized production build...
Failed to compile.

Module not found: Error: Can't resolve './fonts/hack-bolditalic.woff2' in '/Users/se7entyse7en/Projects/.go-workspace/src/github.com/src-d/gitbase-web/frontend/src'


error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
make: *** [Makefile:129: front-build] Error 1

But I noticed that the font file doesn't exist actually. If I remove that font from the less file then the build works. And everything works.

[carlosms]

Please review the original commit (see #158) to see if that file should be there and it was forgotten, or if the problem is that we have that extra include when we should not.

[se7entyse7en]

@carlosms it turned out that there was a missing font. Thanks for pointing to the issue. 👍

[carlosms]

A couple of suggestions

[carlosms]

Let's apply this comment (or a better one if you see a way to explain it more clearly)

[carlosms]

Replace-all? 😄

[se7entyse7en]

🤦‍♂️

[se7entyse7en]

Given that the diff is small I already squashed the commits addressing your comments.

[carlosms]

Sorry, I meant to include this in the previous review.

Instead of using window.location and then discard it, please check if we can do this:

const params = new URLSearchParams();
params.append('query', sql);

return `${rawUrl}?${params.toString()}`;

[se7entyse7en]

It works! And it's much more cleaner this way 👍

[carlosms]

Now I see in travis the tests fail because URLSearchParams is not available in node 🤔
Maybe you can find a polyfill lib and initialize it for the node tests in setupTests.js

[se7entyse7en]

Yup I saw that jest doesn't support URLSearchParams until version 22, and that version comes with react-scripts>=2.0.0.

[carlosms]

Apparently this package has been deprecated and the author recommends to use @ungap/url-search-params instead

[se7entyse7en]

My bad, I haven't noticed it.

[carlosms]

👏 👏 👏