Skip to content

Fix module resolution, Docker setup, and UI issues#261

Merged
DestroyCom merged 10 commits into
masterfrom
fix/pino
May 26, 2026
Merged

Fix module resolution, Docker setup, and UI issues#261
DestroyCom merged 10 commits into
masterfrom
fix/pino

Conversation

@DestroyCom
Copy link
Copy Markdown
Owner

@DestroyCom DestroyCom commented May 26, 2026

Summary by CodeRabbit

  • New Features

    • Added make local-test command for local Docker testing and smoke tests.
  • Bug Fixes

    • Fixed Next.js module resolution issues in containerized environments.
    • Improved application healthcheck timing reliability.
  • Chores

    • Enhanced Docker build workflow and installation of system dependencies.
    • Improved page layouts with flex-based responsive design.
    • Simplified development script workflow.
    • Updated Docker Compose configuration for better local testing support.

Review Change Stack

Make pino runtime files available to the standalone build so worker threads can resolve their modules. next.config.ts marks pino and related packages (pino-pretty, pino-roll, pino-abstract-transport, thread-stream) as serverExternalPackages so Turbopack doesn't bundle them, and the Dockerfile copies the actual pino-abstract-transport package into the flat node_modules in the final image to avoid "Cannot find module 'pino-abstract-transport'" at runtime. pnpm-lock.yaml updated to reflect the dependency changes.
The ab633d1 commit ran pnpm install which dropped the overrides block
from pnpm-lock.yaml while package.json still declares them. This caused
Docker build to fail with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH when using
--frozen-lockfile.
…target

- docker-compose.yml: override SITE_URL=http://localhost:3002, LOG_LEVEL=debug
  and DATABASE_URL pointing into the mounted ./data volume so the SQLite DB
  persists across container restarts
- .gitignore: add logs/ (data/ was already ignored)
- Makefile: add local-test target (mkdir -p guards + compose up with build
  and log streaming)
When the fetch page shows an error with little content, the footer
was floating up instead of sticking to the bottom of the viewport.
Root cause: youtube-dl-exec postinstall runs inside the pnpm cache mount
and can write a 'Not Found' placeholder when the GitHub download is
rate-limited or fails. Subsequent builds use the cached broken binary
→ ENOEXEC on every yt-dlp spawn.

Fix: add an explicit wget step in the deps stage that always overwrites
whatever postinstall put there with the real platform-specific binary,
verified with --version.

Also:
- Remove dead 'node copy-binaries.js' from the dev script (next dev
  never exits so the copy never ran anyway)
- Document in copy-binaries.js that it is for local pnpm start only,
  not Docker (the .next/server/bin path is outside .next/standalone)
The youtube-dl-exec postinstall downloads a pre-compiled PyInstaller
binary (yt-dlp_linux / yt-dlp_linux_aarch64). These are glibc-compiled
and cannot execute on Alpine musl without libc6-compat. On ARM64 builds
(OrbStack on Apple Silicon) the postinstall also picks the wrong
architecture. pnpm's cache mount then freezes the broken binary across
rebuilds.

Fix: install yt-dlp via pip3 in the runner stage. Python3 is already
present (ffmpeg dep), pip3 via py3-pip is architecture-agnostic, and
the pinned version is read from .ytdlp-version (stripping leading zeros
with awk so 2026.03.17 → 2026.3.17 matches PyPI naming). The pip entry-
point script is copied to node_modules/youtube-dl-exec/bin/yt-dlp where
selectYtDlpPath() expects it.

Also removes the failed wget step added in the previous commit.
When the fetch page shows an error or little content, the stroy-500 blue
section ended mid-screen leaving a dark stroy-800 gap before the footer.

- main: add flex flex-col so children can grow vertically
- fetch section: add flex-1 so it fills remaining space in main
submitUrl() calls setIsLoading(true) then router.push() on success but
never calls setIsLoading(false). The component stays mounted across
same-route navigations (searchParams change), so isLoading remained
true indefinitely — causing the search button to stay stuck in
'Recherche en cours...' even after the video loaded.

Fix: useEffect on videoUrl resets isLoading and syncs url state whenever
searchParams.videoUrl changes, i.e. when the navigation has completed.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Warning

Review limit reached

@DestroyCom, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 53 minutes and 45 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 87fe609f-a6df-4b4f-ab53-4eb3a563d2d5

📥 Commits

Reviewing files that changed from the base of the PR and between d11ed00 and 29e83df.

📒 Files selected for processing (2)
  • Dockerfile
  • copy-binaries.js
📝 Walkthrough

Walkthrough

This PR updates Docker image setup to install yt-dlp via pip and fixes Next.js standalone module resolution; adds local development commands and docker-compose environment overrides; adjusts Next.js config to declare external packages; and refactors frontend layout to use flexbox-based responsive design with form input synchronization.

Changes

Build, Runtime, and Frontend Layout Updates

Layer / File(s) Summary
Docker image and yt-dlp installation
Dockerfile
Dockerfile runner stage installs py3-pip and version-pinned yt-dlp, copies the binary into node_modules/youtube-dl-exec/bin/, and fixes Next.js standalone worker-thread resolution by copying real pino-abstract-transport package files from pnpm store into ./node_modules/.
Next.js config and dependencies
next.config.ts, copy-binaries.js
next.config.ts adds pino-abstract-transport and thread-stream to serverExternalPackages and reformats Cache-Control header to multi-line form. copy-binaries.js header documents local standalone usage and Docker build context.
Dev script, local compose, and Makefile
package.json, Makefile, .gitignore, docker-compose.yml
package.json removes copy-binaries step from dev script; Makefile adds docker-up and local-test targets for fresh builds; .gitignore adds data and logs directories; docker-compose.yml adds environment overrides (SITE_URL, LOG_LEVEL, DATABASE_URL) and increases healthcheck start_period to 15s.
Frontend responsive layout and analytics
app/[locale]/layout.tsx, app/[locale]/fetch/page.tsx
layout.tsx updates body to flex column with min-h-dvh and main to flex-grow; adds data-performance="true" to Umami analytics; fetch/page.tsx section adds flex-1 utility class.
Form input state synchronization
components/custom/GetterInput.tsx
Adds useEffect import and effect that watches videoUrl from searchParams, resetting isLoading on change and syncing input url state; reformats isYoutubeUrl to single-line arrow function.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 With yt-dlp now in our Docker build,
And layouts that flex with finesse adorned,
Form states stay sync'd, our flows realized,
Local tests make speedier builds, our path paved true!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix module resolution, Docker setup, and UI issues' accurately summarizes the main changes across the changeset: addressing module resolution (pino-abstract-transport symlink fix), Docker setup (Dockerfile updates with yt-dlp), and UI improvements (Tailwind classes for layout).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pino

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@copy-binaries.js`:
- Around line 8-10: Update the stale header comment in copy-binaries.js that
references Docker downloading yt-dlp in the "deps" stage: change the wording to
state that yt-dlp (and similar Python-installed binaries) are installed in the
Dockerfile's runner stage via pip and therefore the .next/server/bin/ path is
outside .next/standalone and not copied; ensure the comment clearly reflects the
current Dockerfile flow and mentions "runner stage via pip" instead of "deps
stage" so future maintainers are not confused.

In `@Dockerfile`:
- Around line 47-49: The Dockerfile RUN step that computes YTDLP_VERSION and
installs yt-dlp should pass pip's --no-cache-dir to avoid leaving wheel/cache
files in the image; update the pip3 install command in the RUN line (the block
that sets YTDLP_VERSION and calls pip3 install "yt-dlp==${YTDLP_VERSION}") to
include --no-cache-dir so caches aren't persisted in the image layers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bc4a32d8-39b3-4d2e-bdad-d7c124c08f03

📥 Commits

Reviewing files that changed from the base of the PR and between 4d1569b and d11ed00.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (10)
  • .gitignore
  • Dockerfile
  • Makefile
  • app/[locale]/fetch/page.tsx
  • app/[locale]/layout.tsx
  • components/custom/GetterInput.tsx
  • copy-binaries.js
  • docker-compose.yml
  • next.config.ts
  • package.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Always use import { prisma } from '@/lib/prisma' for database access — never instantiate new PrismaClient(). Prisma 7 requires a PrismaLibSql driver adapter; bare instantiation throws at runtime.
Do not call prisma.$disconnect() on the shared singleton instance. Calling disconnect on the singleton breaks subsequent requests and cron runs.

Files:

  • copy-binaries.js
  • components/custom/GetterInput.tsx
  • app/[locale]/fetch/page.tsx
  • next.config.ts
  • app/[locale]/layout.tsx
🪛 Hadolint (2.14.0)
Dockerfile

[warning] 31-31: Pin versions in apk add. Instead of apk add <package> use apk add <package>=<version>

(DL3018)


[warning] 47-47: Avoid use of cache directory with pip. Use pip install --no-cache-dir <package>

(DL3042)

🔇 Additional comments (10)
Dockerfile (1)

31-46: LGTM!

Also applies to: 50-54, 56-70

next.config.ts (1)

8-23: LGTM!

Also applies to: 28-33, 54-55

copy-binaries.js (1)

13-23: LGTM!

package.json (1)

6-6: LGTM!

Makefile (1)

58-68: LGTM!

Also applies to: 102-102

.gitignore (1)

50-51: LGTM!

docker-compose.yml (1)

11-21: LGTM!

Also applies to: 27-27

app/[locale]/layout.tsx (1)

131-131: LGTM!

Also applies to: 135-135, 144-144

app/[locale]/fetch/page.tsx (1)

31-31: LGTM!

components/custom/GetterInput.tsx (1)

6-6: LGTM!

Also applies to: 11-11, 25-32

Comment thread copy-binaries.js Outdated
Comment thread Dockerfile
@DestroyCom DestroyCom merged commit 9e726a0 into master May 26, 2026
4 checks passed
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.

1 participant