-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
fix(studio): fix Docker build #1160
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: b80cefd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for modest-rosalind-098b67 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for asyncapi-studio-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -9,10 +9,10 @@ RUN apk add --no-cache libc6-compat | |||
RUN apk update | |||
# Set working directory | |||
WORKDIR /app | |||
RUN npm install --global turbo | |||
RUN npm install --global turbo@1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to pin the version because the config files here are not compatible with Turbo v2.
@@ -21,32 +21,32 @@ ARG BASE_URL_PLACEHOLDER | |||
RUN apk add --no-cache libc6-compat | |||
RUN apk update | |||
WORKDIR /app | |||
|
|||
RUN npm install --global pnpm@latest-10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As suggested in https://pnpm.io/installation#using-npm
# First install the dependencies (as they change less often) | ||
|
||
COPY .gitignore .gitignore | ||
COPY --from=builder /app/out/json/ . | ||
COPY --from=builder /app/out/package-lock.json ./package-lock.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed anymore.
@@ -28,7 +28,8 @@ | |||
"./src/*", | |||
"./public/*" | |||
] | |||
} | |||
}, | |||
"types": ["cypress"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required so that the Next.js type checker is happy, see https://docs.cypress.io/app/tooling/typescript-support#Configure-tsconfigjson.
b95e196
to
3664fde
Compare
3664fde
to
b80cefd
Compare
Quality Gate passedIssues Measures |
# Build the project | ||
COPY --from=builder /app/out/full/ . | ||
RUN PUBLIC_URL=${BASE_URL_PLACEHOLDER} npm run build:studio | ||
RUN PUBLIC_URL=${BASE_URL_PLACEHOLDER} NEXT_CONFIG_OUTPUT=export pnpm run build:studio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use static HTML export so that it works with the nginx-based setup.
This has some limitations, but none of them seem to apply here at the moment.
A potential future improvement would be to re-engineer the docker setup, but I'd like to get it working again first.
/ptal |
@Amzani @magicmatatjahu @KhudaDad414 @Shurtu-gal Please take a look at this PR. Thanks! 👋 |
Description
The Docker publish workflow has been broken since the migration to Next.js and/or PNPM, see https://github.com/asyncapi/studio/actions/workflows/publish-docker-image.yml.
This MR adapts the Dockerfile to be compatible with the new setup.
Related issue(s)
Discovered while trying to validate #1150
⚒️ with ❤️ by Siemens