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

fix: Improve Docker layer caching on frontend changes #948

Conversation

adamantike
Copy link
Collaborator

Avoid any changes in the frontend/ folder to trigger npm install. Instead, split the copies to have separate steps for install and build.

docker/Dockerfile Outdated Show resolved Hide resolved
Avoid any changes in the `frontend/` folder to trigger `npm install`.
Instead, split the copies to have separate steps for install and build.
@adamantike adamantike force-pushed the fix/improve-docker-layer-caching-on-frontend-changes branch from ccaa8e0 to 32ef612 Compare June 22, 2024 23:29
Copy link
Member

@gantoine gantoine left a comment

Choose a reason for hiding this comment

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

very nice 💯

@gantoine
Copy link
Member

Tried building this on my server and it failed on npm run build. Also tried changing the previous step to npm install and it still failed.

=> ERROR [front-build-stage 6/6] RUN npm run build 14.9s
[front-build-stage 6/6] RUN npm run build:
0.383
0.383 > romm@0.0.1 build
0.383 > npm run typecheck && vite build
0.383
0.550
0.550 > romm@0.0.1 typecheck
0.550 > vue-tsc --noEmit
0.550
14.75 src/App.vue(13,30): error TS7016: Could not find a declaration file for module 'mitt'. '/front/node_modules/mitt/dist/mitt.mjs' implicitly has an 'any' type.
14.75 There are types at '/front/node_modules/mitt/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'mitt' library may need to update its package.json or typings.
14.75 src/components/Administration/Users/Dialog/CreateUser.vue(5,30): error TS7016: Could not find a declaration file for module 'mitt'. '/front/node_modules/mitt/dist/mitt.mjs' implicitly has an 'any' type.
14.75 There are types at '/front/node_modules/mitt/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'mitt' library may need to update its package.json or typings.
14.75 src/components/Administration/Users/Dialog/DeleteUser.vue(7,30): error TS7016: Could not find a declaration file for module 'mitt'. '/front/node_modules/mitt/dist/mitt.mjs' implicitly has an 'any' type.
14.75 There are types at '/front/node_modules/mitt/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'mitt' library may need to update its package.json or typings.
14.75 src/components/Administration/Users/Dialog/DeleteUser.vue(17,38): error TS7006: Parameter 'userToDelete' implicitly has an 'any' type.
14.75 src/components/Administration/Users/Dialog/EditUser.vue(7,30): error TS7016: Could not find a declaration file for module 'mitt'. '/front/node_modules/mitt/dist/mitt.mjs' implicitly has an 'any' type.
14.75 There are types at '/front/node_modules/mitt/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'mitt' library may need to update its package.json or typings.
14.75 src/components/Administration/Users/Dialog/EditUser.vue(18,36): error TS7006: Parameter 'userToEdit' implicitly has an 'any' type.
...

@gantoine gantoine self-assigned this Jun 23, 2024
@adamantike
Copy link
Collaborator Author

I'm not being able to reproduce the error. This is the command I'm using, to disable cache, and pull latest versions for the referenced images, to avoid any existing state. Also building just the frontend target, to validate faster.

docker build --pull --no-cache -t romm:local-frontend --target front-build-stage . --file ./docker/Dockerfile

@gantoine
Copy link
Member

Strange, I run it using the same command and it failed again, with the same error. I'll have a closer look at it later.

@adamantike
Copy link
Collaborator Author

It's possible the .dockerignore is not working as expected, and you have a node_modules folder that gets copied to the container after the npm ci step. I can test that later today, but for now, you can delete that folder and run the build again.

@gantoine
Copy link
Member

you have a node_modules folder that gets copied to the container

yeah that was it! removing it i'm now able to build consistently 🎉

@gantoine gantoine merged commit 2f1daae into rommapp:master Jun 23, 2024
2 checks passed
@adamantike adamantike deleted the fix/improve-docker-layer-caching-on-frontend-changes branch June 23, 2024 17:48
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.

None yet

2 participants