Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(react): set up Docker image for prod and dev #2675

Merged
merged 30 commits into from
Sep 2, 2021

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Aug 25, 2021

Problem

This PRs sets up the Docker containers for both prod and dev.

Development mode

In development mode, docker-compose only builds the backend dependencies such as the server, localstack, and the database.
Since client should (ideally) be completely separate from the server, we can start the frontend independently from the server. This allows for better DX in terms of hot reloading, not needing to rebuild the Docker image when we update npm packages, etc.

To start the server, run Docker as per normal with npm run dev in the root folder (or just docker-compose up)
To start the client, go into /frontend and run npm start.

To view the client, go to localhost:3000. Even though our server will be running on localhost:5000, there is a proxy parameter in frontend/package.json (courtesy of create-react-app) that will proxy all API calls on 3000 to port 5000 automatically. This only happens on development mode.

Production mode

Dockerfile.production has been modified to work and build correctly.
Whilst doing so, encountered a couple of obstacles, such as the styleSrc CSP headers needing to be 'unsafe-inline' due to the usage of emotion. This may be temporary if we want to run a postbuild script that adds a template variable for a nonce into the built index.html that create-react-app creates.

Closes #1904

karrui added 25 commits August 25, 2021 18:13
increases developer friction, containers should remain backend-only, frontend can be started separately and still work due to proxy.
not needed for React branch for now
text debt... too many places in our codebase uses devDependencies...
The backend uses CSP to prevent inline scripts in HTMLs.

By default, Create React App will embed the runtime script into index.html during the production build. When set to false, the script will not be embedded and will be imported as usual. This will allow express to load the script as usual
artifacts from the past, ended up not dockerizing the client
@mantariksh mantariksh requested a review from timotheeg August 30, 2021 06:15
@karrui karrui requested a review from mantariksh August 30, 2021 10:46
# Conflicts:
#	frontend/config-overrides.js
#	package.json
@@ -0,0 +1,86 @@
os: linux
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually why not take this opportunity to use GA for deployment? if not we'll have to migrate again in future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just copy pasted from the old code; still waiting on @tshuli for the GA portion, didn't want to supersede his work

src/app/loaders/express/index.ts Show resolved Hide resolved
src/app/loaders/express/index.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
frontend/package.json Show resolved Hide resolved
@karrui karrui mentioned this pull request Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants