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.
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?
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: https://github.com/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: http://kasheftin.github.io/graphhopper-frontend/index.html
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
After running npm run bundle in the web folder, one giant main.js file is generated containing all dependencies.
Before running npm run bundle, one needs to run npm install to fetch dependencies.
I have removed the checked in dependencies, that could be easily installed with npm. There are still global d3, jQuery, and leaflet objects. This seems to be required for the plugin system to work.
I have not included these front end build steps in the current build infrastructure.
I have migrated the tests gto npm install.
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?
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.
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
npm run bundle
./graphhopper.sh web africa_canary-islands.pbf
No .nvmrc file found
Node Version Manager
Note: refers to any version-like string nvm understands. This includes:
full or partial version numbers, starting with an optional “v” (0.10, v0.1.2, v1)
default (built-in) aliases: node, stable, unstable, iojs, system
custom aliases you define with nvm alias foo
TileLayers gets the correct layer
Expected undefined to be ‘a tile layer’.
Error: Expected undefined to be ‘a tile layer’.
at Object. (/home/peterk/Documents/quell/attic/graphhopper/web/src/test/webapp/spec/config/TileLayersSpec.js:22:23)
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.
This works on my machine …
Actually, it only did when I changed the default tileLayer. It’s fixed now.
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
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