perliedman / lrm-graphhopper

Support for GraphHopper in Leaflet Routing Machine
ISC License
31 stars 52 forks source link

Made it worked but add to modify the source #26

Closed DGrv closed 5 years ago

DGrv commented 5 years ago

Hi,

Great code what you have here. I am a bit a newbie in all those programming javascript so I learn by doing. I finally succeeded to use your code but I had to modify few things. I would like to have your point of view on this and if it is normal.

Except this it is working wonderfully. In which environment require is used ? Maybe my comment are stupid but a small explanation would sastified me :)

psteger commented 5 years ago

There's not enough information here to gather the type of environment you're running this in or what steps you used to install it. If you installed via the npm command listed in the readme, then it should have imported the corslite and polyline dependencies from package.json. Require and import are roughly similar things that allow you to import external modules into your current javascript file. There are differences but I'm not going into them here.

I'm very surprised that this actually works with you removing the require lines (to the point of disbelief). Maybe you mean it's not throwing errors?

My gut feeling is that you grabbed the L.Routing.Graphhopper.js file and manually put it in a project that isn't using Node/NPM then also downloaded corslite and polyline in the same way to get those imports working. Simply running this JS on the frontend without browserify (one of the devDependencies) would probably throw a "require is not defined" error in the console. I could be way off here though. shrug

Without knowing exactly what you're doing, I'm just shooting in the dark here. If I'm right or close, PLEASE do not go this route. NPM is the way to go these days.

perliedman commented 5 years ago

I agree with @psteger, looks like you are trying to run a npm module / CommonJS directly in the browser, which will not work although you can hack around it.

You should either use a pre-built file (although we currently don't do this in this project), or use Browserify/Webpack or similar to bundle the code.

DGrv commented 5 years ago

You are totally right. As I said I have not a lot of knowledge in this field. I did not reply because I wanted to read and try before.

Sorry that I did not give you information before. I wanna use it on my github.pages. In summary I have one html page where I have a leaflet map. I then wanted to use your js directly by sourcing with an url or locally. I then add error regarding the 'require' function. Since I have no real background I am just playing around :p. I remove those lines with 'require' and loaded the wanted .js before. This is how I succeeded to use it.

Thanks a lot for the patient and constructive answer, I learned a lot due to your answer. Just for future reader or for me to understand. The best way is then to install Node.js (and npm). Install the packages needed (as well as browserify) and bundle all and use the output .js. @perliedman has answered I think what I now wrote. Great thanks a lot.