Data4Democracy / internal-displacement

Studying news events and internal displacement.
43 stars 27 forks source link

change webapp structure #134

Closed wwymak closed 7 years ago

wwymak commented 7 years ago

I have changed the 'boilerplate' for the front end app to create-react-app (https://github.com/facebookincubator/create-react-app) that is a bit better documented and more tutorials in general.

I have also split up the web part into client (running the web app) and server (nodejs that handles api calls to interact with the postgres db) so they can be worked on independently.

The client part has some basic views in but not fully fleshed out yet-- but there should be something to see... The nodejs part I have put in a sample api call which does work.

I have separated out the package.json files into client/server/overall web folders -- @WanderingStar let me know if there are any issues with docker if they are structured this way?

To run the app in development mode all you need to do is to call npm run start from the top level folder

WanderingStar commented 7 years ago

So, as a non-Node developer, I don't understand the changes you've made. I've put in a Dockerfile that I think does the appropriate thing, but when I docker-compose up, I get a bunch of errors like

nodejs_1   | [1] /internal-displacement-web/client/src/Api/api.js
nodejs_1   | [1]   18:8  warning  'dummyMapUrl' is assigned a value but never used  no-unused-vars

And when I load http://localhost:3322, I get

Error: ENOENT: no such file or directory, stat '/internal-displacement-web/undefined/index.html'
domingohui commented 7 years ago

It looks like react-scripts of create-react-app uses the port 3000 by default (there's no way to change it apparently?) - it automatically fires up localhost:3000. If I go to 3322, I get the same ENOENT error as @WanderingStar had even if I wasn't running docker-compose.

I think we get the ENOENT error because 3322 is mapped to the server, and the server (the API) is not supposed to be called like that @wwymak ? (there's no index.html)

So I think we should go to localhost:3000 for the front end. 3322 is for API calls.

wwymak commented 7 years ago

@WanderingStar I have updated various bit so web app should run now on localhost:3000 in docker but I think you might need to completely rebuild the web app part for it to run properly as the node modules don't seem to get installed in the right places otherwise.

I think it should be good to go but if you have another look at the docker files that would be great

The warnings in node can be safely ignored as they are from the code linter

WanderingStar commented 7 years ago

When I start the current code up with docker-compose up --build nodejs, I get:

nodejs_1   | [0] [nodemon] starting `node server`
nodejs_1   | [1] npm info 
nodejs_1   | [1] lifecycle id-web-client@0.1.0~start: id-web-client@0.1.0
nodejs_1   | [1] 
nodejs_1   | [1] > id-web-client@0.1.0 start /internal-displacement-web/client
nodejs_1   | [1] > react-scripts start
nodejs_1   | [1] 
nodejs_1   | [0] module.js:472
nodejs_1   | [0]     throw err;
nodejs_1   | [0]     ^
nodejs_1   | [0] 
nodejs_1   | [0] Error: Cannot find module 'body-parser'
nodejs_1   | [0]     at Function.Module._resolveFilename (module.js:470:15)
nodejs_1   | [0]     at Function.Module._load (module.js:418:25)
nodejs_1   | [0]     at Module.require (module.js:498:17)
nodejs_1   | [0]     at require (internal/module.js:20:19)
nodejs_1   | [0]     at Object.<anonymous> (/internal-displacement-web/server/routes.js:8:20)
nodejs_1   | [0]     at Module._compile (module.js:571:32)
nodejs_1   | [0]     at Object.Module._extensions..js (module.js:580:10)
nodejs_1   | [0]     at Module.load (module.js:488:32)
nodejs_1   | [0]     at tryModuleLoad (module.js:447:12)
nodejs_1   | [0]     at Function.Module._load (module.js:439:3)
nodejs_1   | [0] [nodemon] app crashed - waiting for file changes before starting...

Is this (from internal-displacement-web/Dockerfile the intended set of steps to install the node modules?

RUN yarn install
RUN cd /internal-displacement-web/client && yarn install
RUN cd /internal-displacement-web/server && yarn install
WanderingStar commented 7 years ago

The reason we switched from 3000 to 3322 was because we anticipated developers already having something running on the common ports. If this is not actually a concern, it seems fine to leave it running on 3000.

wwymak commented 7 years ago

@WanderingStar did can you check whether the npm install has gone correctly in the subfolders as well? Because body-parser should be specified in the server/package.json file

wwymak commented 7 years ago

Also, I changed the port back to 3000 as that is what the frontend launches by default-- there isn't a config that you can change unless I pull out the run script (which comes from the create-react-app library) into a separate file

WanderingStar commented 7 years ago

Okay, I did a docker system prune and then a docker-compose up to rebuild everything, and it seems to be working now. I've exposed the API port in the docker compose, in case developers want to test direct API calls, but maybe that's unnecessary?

In any case, this seems to work now. I see a UI on http://localhost:3000 and a message on http://localhost:3322

I think it's good to merge to master, if y'all are happy with it.