My impression is that the entire frontend code is contained in the main.js file. This includes both the configuration and business logic. Using a framework like RequireJS, it would be possible to break it into smaller pieces.
My personal use case is that I wanted to patch the code to use a locally running tile server. Currently, I have to insert a few lines of code between lines 400 and 600 of main.js. It would be nice, to insert these lines at the very top of a module config/tileServer.js.
More or less, yes (a util and a client is a bit separated). Feel free to provide PR to improve/substitute this. Not only requirejs would be nice but also knockoutjs and/or npm packaging.
just add the additional layer to the map or if not possible provide also a patch.
GraphHopper UI/demo is indeed not that well engineered like the core 
Thanks for the reply. I had a look around at possible options. There are two, I like:
RequireJS: This is the “old” / battle tested option according to the internet. The main advantage is that it is a modification to the JS code running in the browser without needing anything else installed. The disadvantage is obfuscation. With its “define” blocks, RequireJS add a callback layer to the entire JS code. This is not nice. Furthermore RequireJS forces one to declare the entire packaging in an awkward manner at the beginning of the code.
NPM packaging with browserify. The code looks “nice”. For example one could write “var L = require(‘leaflet’)” instead of including the Leaflet.js script file. The key disadvantage is that it requires NodeJS as a dependency and a frontend build step.
If I were to write a pull request, I would prefer 2. But I’m unsure how much trouble this would be on your side with e.g. updating the https://graphhopper.com/maps/ website.
I think npm packaging is good as it makes components reusable. As a first step I would make our client npm packaged, I tried to separated it a bit more here, than currently done in GH web. But we need stuff like GPX export and more powerful parameter fowarding than done in getParametersAsQueryString etc
Maybe this can be used instead? I’ve not looked deep inside if it already supports all of our features like geometry with optional elevation etc but maybe it is wiser to use and contribute to this than to implement it again (?) (I like Apache License also a bit more but this is okay)
I’ve started extracting a bit of code into packages at https://github.com/HelgeKrueger/graphhopper
So far, I’ve extracted the logic creating the map and the instructions. Unfortunately, this is currently far from good looking as I’m mostly cutting code blocks from one file to another and then trying to fix errors that arose.
Regarding the GraphHopperRouting.js implementation. This seems to be similar to what is currently in the web implementation as GHRequest. Is this correct?
Yes, probably just stick to this 
Your work seems to be in the correct direction (pun intended ;)) and I highly appreciate this! I once started the UI as a very simplistic demo without thinking for future maintenance and now it is time to throw it away or refactor it. In case you think the first option is better - let me know. Also I forgot to tell you about the work done here: GitHub - Kasheftin/graphhopper-frontend Kasheftin did also a bit modernization of the UI itself with knockoutjs and implemented the “dragging from a line” seen in other popular online route planners already. But I never found time to include these features or our other features there so it got sadly in a stale state. Try it here: GraphHopper frontend
As I remember from some of my previous ‘investigations’: the npm packaging and require will at the end also enable us to minify the code too, and use one js for production although we use several JS files for development - is this correct?
As we have also a very tiny test coverage in the demo, feel free to add stuff there too, if the refactoring breaks code without notifying us in these tests.
I think I’m so far done, that no big code moving around actions have to be done anymore 
One thing, I’ve noticed while refactoring is that the autocomplete functionality for location names is broken when running a vanilla local installation. This is due to “ghRequest.createGeocodeURL” would return “localhost/geoCode” as endpoint to use. Now, the geoCode endpoint is not part of the installation. In order to have it work, one needs to add a valid API key to ghRequest and replace host with graphhopper.com.
One probably wants to change this, that it always works. Maybe even use this implementation of the API call, but the API key seems to missing from it.
I think the only remaining shop stopper for me posing a pull request is the integration of the buildstep in the current build process. One probably wants to ensure that npm is installed in graphhopper.js, and then trigger the npm build steps with maven?
Nice, this looks like a big and nice step into better maintainability - thanks!
Nice, I’ll try it out in the next days! Do you have a simple step by step guide for me as a npm newby ?
I have not included these front end build steps in the current build infrastructure.
You mean with the maven stuff? I can also do a bit of research, there is also the missing bit that the JS tests should be executed from maven and there was a plugin which does this…
One thing, I’ve noticed while refactoring is that the autocomplete functionality for location names is broken when running a vanilla local installation
Geocoding is only part of the demo, yes.
Yes, we could point to graphhopper.com/api/1 and ask developer for registration, we cannot provide one single free API key - we’ve learned this from history
Here’s a quick introduction on what to do:
Get node installed. I used nvm
curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.29.0/install.sh | bash
nvm install 4.2.2
nvm use 4.2.2
Clone my repo git clone https://github.com/HelgeKrueger/graphhopper.git.
Go to graphhopper/web and run
nvm install
nvm test
nvm run bundle
Then go to graphhopper and run ./graphhopper.sh web africa_canary-islands.pbf.
The npm config is in package.json. The folder node_modules contains the modules downloaded with nvm install. To install new ones use nvm install --save module_name.
That’s hopefully all you need to do. It worked on my machine 
.
There are still plenty of very rough edges on what I have done so far. I think that smoothing them out is out of scope for what I have done so far. Fortunately, I HOPE that this is now a local task. The next few people having problems with the front end code should be able to smooth them out. I might even be the one doing it. 
I’ve now changed the implementation of AutoComplete to suggest to developers to do this.
Thanks for this intro 
! I’ll play with it!
What about distribution? I would prefer if we still ship a ‘compiled’ version to avoid that users have to do this - what do you think about this?
Thanks again for your work towards this cleaner structure !
Here is my final install script *
Are the last commands really nvm? I used npm instead as nvm did not work **
One test seems to fail ***
To install new ones use nvm install --save module_name.
What do you mean with this? (sorry if this is a stupid question ;))
Another problem I stumbled over is when I drag a marker it disappears (it says autocomplete.hide is not a function) ie. we need a test for this but I’m really unsure how one would do such UI related tests - do you have an idea?
And how is it possible to easier debug this minified JS? E.g. is it possible to create an ‘exploded’ / pretty main.js ?
BTW: maybe even unminified in general? as the savings are less than I hoped: 680KB vs. 740KB or did I not properly look into firebug?
curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.29.0/install.sh | bash
# close and reopen terminal now
nvm install 4.2.2
nvm use 4.2.2
git clone https://github.com/HelgeKrueger/graphhopper.git
cd graphhopper/web
npm install
npm test
npm run bundle
cd ..
./graphhopper.sh web africa_canary-islands.pbf
firefox localhost:8989
**
No .nvmrc file found
Node Version Manager
Note: refers to any version-like string nvm understands. This includes:
nvm alias foo***
Failures:
Say you wanted to add the underscore library to the project. Then you would use npm install --save underscore command. This command automatically modifies package.json to include underscore and a version info. This means that the next person running nvm install would also download underscore.
That was me not paying attention when I redid AutoComplete. It’s fixed now.
I’ve removed the minification from the bundle command and added the bundleUgly and bundleDebug command. bundleDebug adds source maps, which helps the debugger.
It’s a pragmatic idea. If this is ok, I would add the pull request for this code soon. I like this. The disadvantage is that this means that the process of modifying the JavaScript code would be somewhat awkward.
This works on my machine …
Actually, it only did when I changed the default tileLayer. It’s fixed now.
This is now available as pull request https://github.com/graphhopper/graphhopper/pull/590 and will be merged soon - thanks @HelgeKrueger !
Is it possible somehow to specify variables which are different for local installation vs. deployment? E.g. I need to change the API key and the host for GraphHopper Maps and before the change I did the deployment via a simple text replacement tool (sed) which is now no longer working
I think it would probably be best to add config file for these things, which creates a config object. Which variables does one need to change. I’m aware of:
And how would I bundle the config or tileLayers object together to create a different main.js?
Would something like
browserify src/main/webapp/js/main-template.js config.js -o src/main/webapp/js/main.js
work?