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.

Sign up for GitHub

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 TS in Docker, not when starting app #1008

Merged
merged 10 commits into from
Feb 17, 2023
Merged

Build TS in Docker, not when starting app #1008

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ee2bef3
Build TS in Docker, not start
Feb 15, 2023
d2a28af
e2e
Feb 15, 2023
ab6df3c
Merge branch 'main' into shayne-build-ts-in-dockerfile
Feb 16, 2023
0c1b067
update e2e after main merge
Feb 16, 2023
20fd2f7
refactoring Dockerfile
Feb 16, 2023
39c2c62
copy from builder instead of local
Feb 16, 2023
d03df69
update e2e
Feb 16, 2023
b7c0e5a
added changelog message
Feb 16, 2023
9710b47
update comment
Feb 17, 2023
d336d89
update changelog
Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions waspc/data/Generator/templates/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ WORKDIR /app
COPY --from=server-builder /app/server/node_modules ./server/node_modules
infomiho marked this conversation as resolved.
Show resolved Hide resolved
COPY server/ ./server/
COPY db/ ./db/
RUN cd server && npm run build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't run this in server-builder since I need the source code, which is copied over here. I didn't want to fundamentally restructure this Dockerfile, and instead do the smallest change that will work, since we will rethink this process in the near future.

Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand this!
So we need to build a TS project. Why not build it in server-builder? I guess you would need to copy some extra files then, but I guess that wouldn't be too hard? I don't think this is too bad either, it is probably a small impact, but I am curious.

Copy link
Contributor

@infomiho infomiho Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There is an added benefit if we did it in the server-builder step, we could copy only the node_modules and dist folder to the next step (avoiding copying over the src and everything else).

But we should still separate the copying of src and package*.json files to keep the layer caching benefits.

FROM base AS server-builder
...
COPY server/package*.json ./server/
RUN cd server && npm install
...
COPY server/ ./server/
RUN cd server && npm run build

FROM base AS server-production
...
COPY --from=server-builder /app/server/node_modules ./server/node_modules
COPY --from=server-builder /app/server/dist ./server/dist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok ok, you all convinced me, I will just refactor it a bit to do all the building where it probably should. The 1 line fix was just so appealing though :D

EXPOSE ${PORT}
WORKDIR /app/server
ENTRYPOINT ["npm", "run", "start-production"]
Expand Down
4 changes: 2 additions & 2 deletions waspc/e2e-test/test-outputs/waspBuild-golden/waspBuild/.wasp/build/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspBuild-golden/waspBuild/.wasp/build/Dockerfile
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspBuild-golden/waspBuild/.wasp/build/server/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions waspc/e2e-test/test-outputs/waspCompile-golden/waspCompile/.wasp/out/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspCompile-golden/waspCompile/.wasp/out/Dockerfile
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspCompile-golden/waspCompile/.wasp/out/server/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions waspc/e2e-test/test-outputs/waspJob-golden/waspJob/.wasp/out/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspJob-golden/waspJob/.wasp/out/Dockerfile
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspJob-golden/waspJob/.wasp/out/server/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions waspc/e2e-test/test-outputs/waspMigrate-golden/waspMigrate/.wasp/out/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspMigrate-golden/waspMigrate/.wasp/out/Dockerfile
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspMigrate-golden/waspMigrate/.wasp/out/server/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/src/Wasp/Generator/ServerGenerator.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ genPackageJson spec waspDependencies = do
"npmVersionRange" .= show npmVersionRange,
"startProductionScript"
.= ( (if hasEntities then "npm run db-migrate-prod && " else "")
++ "NODE_ENV=production npm run build-and-start"
++ "NODE_ENV=production npm run start"
),
"overrides" .= getPackageJsonOverrides
]
Expand Down