Skip to content

Comments

feat: setup docker schema#75

Open
MartinFillon wants to merge 2 commits intomainfrom
feat/setup-docker-schema
Open

feat: setup docker schema#75
MartinFillon wants to merge 2 commits intomainfrom
feat/setup-docker-schema

Conversation

@MartinFillon
Copy link
Contributor

What does this PR do?

creates a docker schema to generate a dockerfile

How do you test this PR?

e2e tests

closes #74

@MartinFillon MartinFillon self-assigned this Feb 24, 2026
@MartinFillon MartinFillon requested a review from Exeloo as a code owner February 24, 2026 10:12
Copy link
Member

Choose a reason for hiding this comment

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

.template isn't usefull

Copy link
Member

Choose a reason for hiding this comment

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

.template isn't usefull

COPY bun.lock ./
<% } %>

RUN <%= packageManager %> install --frozen-lockfile
Copy link
Member

Choose a reason for hiding this comment

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

Not possible anymore as npm will depreciate --frozen-lockfile in next version

Image

For npm use --no-package-lock instead


RUN <%= packageManager %> run build

CMD ["<%= packageManager %>", "start"]
Copy link
Member

Choose a reason for hiding this comment

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

Run scripts without run command isn't available on npm

Suggested change
CMD ["<%= packageManager %>", "start"]
CMD ["<%= packageManager %>", "run", "start"]

Comment on lines +7 to +11
<% if (packageManager === 'pnpm') { %>
ENV PNPM_HOME="/pnpm"
ENV PATH="$PNPM_HOME:$PATH"
RUN npm install -g pnpm
<% } %>
Copy link
Member

Choose a reason for hiding this comment

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

Missing corepack installation for pnpm

Comment on lines +17 to +27
WORKDIR /app
COPY package.json ./
<% if (packageManager === 'npm') { %>
COPY package-lock.json ./
<% } else if (packageManager === 'yarn') { %>
COPY yarn.lock ./
<% } else if (packageManager === 'pnpm') { %>
COPY pnpm-lock.yaml ./
<% } else if (packageManager === 'bun') { %>
COPY bun.lock ./
<% } %>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be better to create a variable in the factory and use it here instead of this 4 branches condition

applyTemplates({
...strings,
...options,
dot: ".",
Copy link
Member

Choose a reason for hiding this comment

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

Unused variable

@@ -0,0 +1,28 @@
{
"$schema": "http://json-schema.org/draft-07/schema",
"$id": "SchematicsNanoForgeBaseApplication",
Copy link
Member

Choose a reason for hiding this comment

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

Bad id

},
"docker": {
"factory": "./libs/docker/docker.factory#main",
"description": "Create a Main file for client or server.",
Copy link
Member

Choose a reason for hiding this comment

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

Bad description


it("should create docker file with install command", () => {
const content = tree.readContent(`/${packageManager}-test-app/Dockerfile`);
expect(content).toContain(`RUN ${packageManager} install --frozen-lockfile`);
Copy link
Member

Choose a reason for hiding this comment

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

This tests isn't valid anymore as npm will depreciate --frozen-lockfile in next version

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.

Create docker schematics

2 participants