Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

Improve Node, Docker, Postegres and Other Sections of CONTRIBUTING.md #447

Closed
8 tasks done
allella opened this issue Mar 8, 2021 · 11 comments
Closed
8 tasks done
Assignees
Labels
Documentation Improvements or additions to documentation

Comments

@allella
Copy link
Contributor

allella commented Mar 8, 2021

Jonathan and Tom pointed out in the meeting that Node 13 is not longer supported. The Node release page suggests the same since Node 13 is not an LTS release and was only supported for 6 months.

It seems this Node 13 version was also causing issues with the package.json configuration of npm and node.

In addition, I'm going to work through the CONTRIBUTING.md and get everything running locally with a "fresh set of eyes" since I have yet to actually fire up the server.

A lot of new contributors have been hung up on Docker, so we'll see if any of the recent devs have improvements on the Docker side.

There are other consistency and wording changes to be made.

If anybody has suggests on improvements, then feel free to shared and get involved.

List of updates to make

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

I'm using git bash on windows and I think I read something somewhere that yarn has some issues on this platform. When I would run "npm run both" I would a different error message each time relating to typeorm. To get around this, I replaced yarn with npm in package.json:
"both": "concurrently "yarn dev" "yarn --cwd='./client' dev"",
to
"both": "concurrently "npm run dev" "cd client & npm run dev"",

Then I was able to get the local (non Docker) setup working with "npm run both"

I'm nowhere near guru level when it comes to node/npm so this was just me following the trail of error messages, research and trying things until it worked.

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

I haven't been able to get the Docker setup working yet. When I try, everything seems to go ok but the browser console shows CORS errors when attempting to connect to the app.

Does anyone else get these CORS errors or is it just me/my platform?

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

I'm up to date with the node and npm versions.
$ node -v
v14.15.4
$ npm -v
6.14.10

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

@paizle what browser is showing the CORS error? That's a security feature and "localhost" is a unique domain, so it could be a specific browser tweak or that we need to add some CORS headers to the app to bypass the localhost issue.

When you run Docker does it have these success lines as part of the output, as shown in this PR in the Running the App -> Step 2 > Docker Mode section?

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

db_1 | ... LOG: database system is ready to accept connections

client_1 | ready - started server on http://localhost:3000

app_1 | Listening on http://localhost:5000/graphql

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

Another thought, is the "both" command is using a \ escape character to add a literal double quote to the package.json and that's occasionally a source of edge cases.

Could you try, instead of running "yarn both" to run the individual commands it runs to check the escaping isn't confusing other tools?

yarn dev
yarn build:client

@allella
Copy link
Contributor Author

allella commented Mar 9, 2021

@paizle the "escape character" not playing nice with git bash and yarn seems plausible, especially if running the two commands separately fixes your issue.

If that is the issue we may be able to redo the "both" command, but let's see if running the commands separately does the trick.

yarn dev
yarn build:client

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

@paizle what browser is showing the CORS error? That's a security feature and "localhost" is a unique domain, so it could be a specific browser tweak or that we need to add some CORS headers to the app to bypass the localhost issue.

When you run Docker does it have these success lines as part of the output, as shown in this PR in the Running the App -> Step 2 > Docker Mode section?

I do get those success lines.

With the yarn db:reset command I get the following:
C:\Users\Matt\Documents\workspace\chapter\clones\another\Chapter\node_modules.b
in\typeorm:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\,/,g')")

...but I can get around this issue by changing the root package.json:
"typeorm": "ts-node -P tsconfig.server.json ./node_modules/.bin/typeorm",
to:
"typeorm": "ts-node -P tsconfig.server.json ./node_modules/typeorm/cli.js",
(I think this is another issue with git bash on windows)

So, finally, after I run docker-compose up it's working this time. (yay! no CORS issues) I think somehow the docker setup was connecting to my local postgres because I explicitly shut down the service this time but I'm not sure if that's true since I don't have a good grasp on how docker works and if that was causing the issue.

@paizle
Copy link
Contributor

paizle commented Mar 9, 2021

Another thought, is the "both" command is using a \ escape character to add a literal double quote to the package.json and that's occasionally a source of edge cases.

Could you try, instead of running "yarn both" to run the individual commands it runs to check the escaping isn't confusing other tools?

yarn dev
yarn build:client

Ok I tried both and here are the results (just what seems to be the relevant output):

$ yarn dev
yarn run v1.22.5
$ ts-node-dev --no-notify -P tsconfig.server.json ./server/app.ts
ts-node-dev ver. 1.0.0-pre.63 (using ts-node ver. 8.10.2, typescript ver. 3.9.7)
(node:4128) UnhandledPromiseRejectionWarning: Error: Entity metadata for Event#s
ponsors was not found. Check if you specified a correct entity object and if it'
s connected in the connection options.
$ yarn build:client
yarn run v1.22.5
$ yarn --cwd='./client' build
error Command "build" not found.

No luck with either. I don't use yarn much so I don't know a lot about it.

@allella
Copy link
Contributor Author

allella commented Mar 25, 2021

#461 aims to remove yarn, so hopefully that works around the git bash + yarn issues we talked about trying to document around.

@allella
Copy link
Contributor Author

allella commented Apr 1, 2021

Closing this as the core items have been improved.

There are a number of troubleshooting errors I've encountered on Windows with Docker and Node, so I'll start a new issue with Troubleshooting topics to cover.

@allella allella closed this as completed Apr 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants