Fix yalc publish

[Judahmeek]

Yalc Publish has been failing since Sep 29th.

Here's the fix.

Summary by CodeRabbit

• Chores
  • Improved build script artifact detection to more reliably recognize prebuilt artifacts and to adjust when a build is skipped or retried, reducing false negatives during automated builds and improving overall build consistency.

[coderabbitai]

Walkthrough

The build script in package-scripts.yml changes two conditional checks to look for lib/ReactOnRails.full.js instead of packages/react-on-rails/lib/ReactOnRails.full.js, altering the artifact-detection path used to decide whether a prebuilt artifact is present.

Changes

Cohort / File(s)	Change Summary
Build Configuration package-scripts.yml	Replaced references to packages/react-on-rails/lib/ReactOnRails.full.js with lib/ReactOnRails.full.js in two conditional checks that detect a prebuilt build artifact (affects skip/failure detection paths)

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Developer/CI
    participant Script as package-scripts.yml
    participant FS as File System

    Dev->>Script: start build
    Script->>FS: test -f lib/ReactOnRails.full.js
    alt file exists
        FS-->>Script: file found
        Script->>Dev: skip build (prebuilt artifact detected)
    else file missing
        FS-->>Script: file not found
        Script->>Dev: run npm build or mark failure
    end
    note right of Script `#DDDDFF`: Two conditional checks updated to use the shorter path

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify the updated path is correct relative to the script's working directory and runtime environment.
• Confirm both conditional checks cover the intended locations and that skipping/failure behavior remains correct.
• Run a local CI or dry-run to ensure artifact detection behaves as expected in typical workflows.

Poem

🐰 I hopped through branches, trimmed a long trail,
Now lib/ReactOnRails.full.js shines like a nail.
Shorter and spry, the check finds its prize,
A nibble, a build—then celebratory sighs!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix yalc publish' directly relates to the PR's primary objective of fixing a failing Yalc publish process by updating build artifact path references.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch judahmeek/yalc

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6236ae3 and 278adde.

📒 Files selected for processing (1)

• package-scripts.yml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• package-scripts.yml

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review - PR #2054: Fix yalc publish

Summary

This PR fixes a critical bug in the yalc publish workflow that has been broken since September 29th. The fix corrects the file path check in the prepack script.

---

✅ Approved - Excellent Fix

What This PR Does

The PR corrects the file path in package-scripts.yml from packages/react-on-rails/lib/ReactOnRails.full.js to lib/ReactOnRails.full.js.

Why This Fix Is Correct

Root Cause Analysis:

1. When running yarn yalc:publish from the root, it calls each workspace's yalc:publish script
2. The workspace packages/react-on-rails/package.json has "prepack": "nps build.prepack"
3. When nps (node-package-scripts) runs from within the workspace directory (packages/react-on-rails/), it finds package-scripts.yml at the monorepo root
4. However, the working directory when the script executes is the workspace directory (packages/react-on-rails/)
5. Therefore, the script must check for lib/ReactOnRails.full.js (relative to workspace), not packages/react-on-rails/lib/ReactOnRails.full.js (which would be relative to root)

Verification from package.json:

• The workspace's main field is: "main": "lib/ReactOnRails.full.js" (packages/react-on-rails/package.json:5)
• This confirms the build output is at packages/react-on-rails/lib/ReactOnRails.full.js in absolute terms
• But relative to the workspace directory where the script runs, it's just lib/ReactOnRails.full.js

Code Quality: ✅ Excellent

• Minimal, targeted fix
• Addresses the exact root cause
• No unnecessary changes
• Symmetric: both file checks updated consistently

Testing: ⚠️ Recommendation

While the fix is clearly correct based on code analysis, I recommend:

1. Manual verification: Run yarn yalc:publish from the root to confirm it works
2. Test the prepack hook specifically:

   cd packages/react-on-rails
   yarn build  # Ensure lib/ReactOnRails.full.js exists
   yarn pack   # This triggers prepack hook

Impact Assessment

• Severity: Critical (blocks local development workflow)
• Scope: Affects all contributors using yalc for local testing
• Risk: Very low - this is a path correction with no logic changes
• Breaking Changes: None

Security: ✅ No Concerns

No security implications.

Performance: ✅ No Impact

No performance changes.

Documentation Alignment: ✅

This fix restores functionality documented in:

• CONTRIBUTING.md (lines 100-103, 113, 120-139)
• CLAUDE.md (line 359)
• docs/contributor-info/errors-with-hooks.md (lines 37-44)

---

Final Recommendation: ✅ APPROVE & MERGE

This is a clear, correct fix for a critical development workflow issue. The change is minimal, well-targeted, and restores documented functionality.

Suggested Merge Message:

Fix yalc publish prepack path check

The prepack script in package-scripts.yml was checking for 
packages/react-on-rails/lib/ReactOnRails.full.js but when nps runs 
from within the workspace directory, the correct path is 
lib/ReactOnRails.full.js (relative to the workspace).

This restores the yalc publish workflow for local development.

---

Review generated by Claude Code following CLAUDE.md guidelines

[claude]

Code Review - PR #2054: Fix yalc publish

Summary

This PR fixes the yalc publish functionality that has been broken since September 29th by correcting the path to build artifacts in the prepack script.

Changes Analysis

File: package-scripts.yml

• Changed path from packages/react-on-rails/lib/ReactOnRails.full.js to lib/ReactOnRails.full.js
• Affects the build.prepack script that runs during prepack and prepare lifecycle hooks

✅ Positive Findings

1. Root Cause Correctly Identified: The path was indeed incorrect. When npm/yarn runs the prepack script defined in packages/react-on-rails/package.json, the working directory is packages/react-on-rails/, not the workspace root.

2. Minimal Change: The fix is surgical - only changes what's necessary to fix the issue.

3. Consistent: Both occurrences of the path (check before build and check after build) are updated consistently.

4. Script Logic Preserved: The conditional build logic remains intact:

   • Skip build if already built
   • Build silently (ignore missing devDeps errors)
   • Verify build succeeded
   • Print error if verification fails

🔍 Verification Needed

1. Testing: Has this been tested locally with yalc publish to confirm it works?

2. When Did This Break?: The PR mentions Sep 29th - was there a structural change to the monorepo or package.json that caused the previous path to stop working?

3. Related Scripts: Should verify that other scripts in package-scripts.yml don't have similar issues.

💡 Suggestions

Consider adding a comment to clarify the path context:

# Note: This script runs from packages/react-on-rails/ context during prepack
script: >
  [ -f lib/ReactOnRails.full.js ] ||
    (npm run build >/dev/null 2>&1 || true) &&
    [ -f lib/ReactOnRails.full.js ] ||
    { echo 'Building react-on-rails seems to have failed\!'; }

🎯 Overall Assessment

Approval Status: ✅ LGTM (Looks Good To Me)

The fix is correct and addresses the immediate issue. The change makes sense given that npm lifecycle scripts run in the package directory context, not the workspace root.

Recommendation:

• Approve and merge after confirming local testing
• Consider documenting this gotcha in CONTRIBUTING.md or CLAUDE.md to prevent similar issues in the future

Questions

• Have you tested this locally with yarn yalc:publish?
• Should we add a CI check that validates yalc publish works?

[claude]

Updated Code Review - PR #2054: Fix yalc publish

I need to provide a clarification on my previous review after re-examining the workspace execution context.

Correction: This Fix Is Actually CORRECT ✅

After reviewing the documentation (CONTRIBUTING.md:111-113), I can confirm that yalc commands are run from within the workspace directory (packages/react-on-rails/), not from the monorepo root.

Key Evidence:

1. CONTRIBUTING.md shows: cd <React on Rails root>/packages/react-on-rails then yalc push
2. When yalc publish runs, the current working directory is packages/react-on-rails/
3. The prepack script executes in this context, so paths must be relative to the workspace
4. The workspace's package.json shows: "main": "lib/ReactOnRails.full.js" (line 5) - relative to workspace

Why The Original Path Was Wrong

Before (broken):

[ -f packages/react-on-rails/lib/ReactOnRails.full.js ]

When running from packages/react-on-rails/, this checks for:
packages/react-on-rails/packages/react-on-rails/lib/ReactOnRails.full.js ❌ (doesn't exist)

After (correct):

[ -f lib/ReactOnRails.full.js ]

When running from packages/react-on-rails/, this checks for:
packages/react-on-rails/lib/ReactOnRails.full.js ✅ (correct location)

Final Recommendation: ✅ APPROVE & MERGE

This fix correctly restores the yalc publish functionality by using workspace-relative paths that match the execution context.

---

Apologies for the confusion in my initial review - I misunderstood the execution context. This PR is correct and should be merged.