Conversation
53586f6 to
1f568b5
Compare
Make sure patches are applied in Docker builds and optional deps are available. This broke in combination of #14966 and #15729. The issue is: - Patches were no longer applied because of an issue in #14966 - After #15729 optional dependencies are also required, which were removed in #14966 Fixes #16281
1f568b5 to
de32453
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
cubic reviewed 1 file and found no issues. Review PR in cubic.dev.
| RUN NODE_ENV=production DOCKER_BUILD=true pnpm --filter=n8n --prod --no-optional --legacy deploy /compiled | ||
| # We don't want to use --no-optional because pdfjs-dist has an optional dependency on | ||
| # @napi-rs/canvas, which is required for the pdfjs-dist package to work. | ||
| RUN NODE_ENV=production DOCKER_BUILD=true pnpm --filter=n8n --prod --legacy deploy /compiled |
There was a problem hiding this comment.
have you tested this? when I tried @napi/canvas did get installed, but failed to load because of missing binaries.
There was a problem hiding this comment.
I did, and it did work after this. It seemed that the package was indeed missing. But the binary was also missing unless I specified linux/amd64 platform specifically. I can try and see if it's needed
There was a problem hiding this comment.
Okay tested it with the --no-optional and it does not work. So we need to remove it
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
There was a problem hiding this comment.
It works when built for amd64.
One concern is the docker image size increase though.
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
|
Got released with |
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>
Summary
Make sure patches are applied in Docker builds and optional deps are available.
This broke in combination of #14966 and #15729. The issue is:
Including optional dependencies increases the
node_modulessize from915.5Mto1.0GRelated Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/CAT-956/community-issue-binary-files-broken-in-version-1980-next
Fixes #16281
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)