Skip to content

Conversation

@simonkberg
Copy link
Owner

Moves @next/third-parties from devDependencies to dependencies, as it's used at runtime via next/third-parties/google.

Closes #1548

Generated with Claude Code

This package is used at runtime via next/third-parties/google,
so it should be a production dependency rather than a dev dependency.

Co-authored-by: Simon Kjellberg <simonkberg@users.noreply.github.com>
@claude
Copy link
Contributor

claude bot commented Jan 28, 2026

Claude finished @simonkberg's task in 1m 17s —— View job


Code Review Complete ✓

Tasks

  • Read changed files (package.json, pnpm-lock.yaml)
  • Verify runtime usage of @next/third-parties
  • Review against code quality criteria
  • Check security implications
  • Verify testing implications
  • Post comprehensive review feedback

Review Summary

Verdict: ✅ Approved - This is a correct and necessary fix

This PR correctly moves @next/third-parties from devDependencies to dependencies. The package is used at runtime in app/components/Layout.tsx:3 via the GoogleTagManager component, which renders on every page of the site.

Detailed Analysis

1. Code Quality ✅

Strengths:

  • Correct dependency classification: The package is imported and used in Layout.tsx which is part of the runtime application bundle
  • Clean commit message: Clear explanation of why the change is needed
  • Minimal impact: Only touches dependency declarations, no code changes required

Verification:

// app/components/Layout.tsx:3
import { GoogleTagManager } from "@next/third-parties/google";

The Layout component wraps all pages and uses GoogleTagManager at runtime, making this a production dependency.

2. Security ✅

No security concerns identified:

  • No change to package version (16.1.6) - only moving between dependency sections
  • Official Next.js package from Vercel (@next namespace)
  • No new code or attack vectors introduced

3. Performance ✅

Build Impact:

  • Positive: With output: "standalone" in next.config.ts:6, Next.js bundles only production dependencies into the standalone build
  • Before this fix: The package might have been missing from production builds, potentially causing runtime errors
  • After this fix: Package is correctly included in Docker production builds (Dockerfile:15 uses pnpm i --frozen-lockfile)

4. Testing ✅

Test coverage verified:

  • Existing test in app/components/Layout.test.tsx:33-43 validates GoogleTagManager rendering
  • Test checks for GTM script injection with correct ID
  • No test changes needed - tests already mock the component behavior and don't directly import from @next/third-parties
// Layout.test.tsx:33-43 already tests GTM rendering
it("should include Google Tag Manager with correct ID", async () => {
  render(<Layout />, { container: document });
  expect(document.body.querySelector("script[id=_next-gtm-init]")).toBeInTheDocument();
  // ...
});

5. Documentation ✅

No documentation updates needed:

  • Change is internal (dependency classification)
  • No API changes or new features
  • PR description clearly explains the rationale

Additional Observations

Docker Build Correctness

This fix ensures correct behavior in the Docker production build:

  1. deps stage (Dockerfile:15): pnpm i --frozen-lockfile installs all dependencies
  2. builder stage (Dockerfile:28-33): Copies node_modules and builds with output: "standalone"
  3. runner stage (Dockerfile:43): Only standalone bundle is copied (which now correctly includes @next/third-parties)

Without this fix, the standalone build might have excluded @next/third-parties from the production bundle, causing runtime errors when GoogleTagManager tries to load.

pnpm-lock.yaml Changes

The lockfile changes (pnpm-lock.yaml:+3/-3) correctly reflect the dependency move:

  • Package remains at the same version
  • Only the dependency graph location changed (from devDependencies to dependencies)
  • No transitive dependency changes

Recommendation

This PR should be merged. It fixes a bug where a runtime dependency was incorrectly classified as a dev dependency. The change is:

  • ✅ Correct and necessary
  • ✅ Minimal and focused
  • ✅ Well-tested (existing tests cover the functionality)
  • ✅ Safe (no version changes, no new code)

@simonkberg simonkberg merged commit 7cd396b into main Jan 28, 2026
5 checks passed
@simonkberg simonkberg deleted the claude/issue-1548-20260128-2011 branch January 28, 2026 20:28
@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.76%. Comparing base (be7cb83) to head (e331755).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1549   +/-   ##
=======================================
  Coverage   97.76%   97.76%           
=======================================
  Files          49       49           
  Lines         896      896           
  Branches      156      156           
=======================================
  Hits          876      876           
  Misses          8        8           
  Partials       12       12           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

Make @next/third-parties a production dependency

2 participants