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

Refactor docker, fix unhandled error #64

Merged
merged 4 commits into from
Oct 1, 2023
Merged

Refactor docker, fix unhandled error #64

merged 4 commits into from
Oct 1, 2023

Conversation

Siegrift
Copy link
Collaborator

@Siegrift Siegrift commented Oct 1, 2023

I wanted to work on #52 but realized it would be better to fix the Docker images after the refactor in #51.

I also noticed an unhandled error and improved logging messages.

Single Dockerfile in monorepo root

I realized that with the addition of common package, the previous approach does not work. The fix would be very similar to https://pnpm.io/docker#example-2-build-multiple-docker-images-in-a-monorepo so I decided to go with their suggestion. In case we need to add Redis (or other container) we can use docker-compose to run the containers together and still use the common Dockerfile to built the packages. For example:

version: '3'

services:
  api:
    build:
      context: ../../  # Path to your monorepo root, where the Dockerfile is located
      target: api  # Target stage in your Dockerfile
    ports:
      - "8080:8080"
    depends_on:
      - redis
  redis:
    image: redis:latest
    ports:
      - "6379:6379"

Unfortunately, their example was broken and their documentation around monorepo + docker was not very good. In the end I managed to get it working. You can read the details in the Dockerfile (inline) docs. LMK if there is something unclear.

Removed source map support

I realized that we don't need the package for some time (after node 12). There is a note though about source maps not working correctly when using vm package which we use for pre/post processing. For these the source maps will not work.

@Siegrift Siegrift requested a review from andreogle October 1, 2023 18:16
@Siegrift Siegrift self-assigned this Oct 1, 2023
@Siegrift Siegrift merged commit 4fe829c into main Oct 1, 2023
3 checks passed
@Siegrift Siegrift deleted the refactor-docker branch October 1, 2023 18:27
@Siegrift Siegrift mentioned this pull request Oct 20, 2023
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.

1 participant