Skip to content

feat: Adds in skeleton recipe pre-commit hook#3500

Open
andguy95 wants to merge 11 commits intomainfrom
an-feat-skeleton-recipe-commit-hook
Open

feat: Adds in skeleton recipe pre-commit hook#3500
andguy95 wants to merge 11 commits intomainfrom
an-feat-skeleton-recipe-commit-hook

Conversation

@andguy95
Copy link
Collaborator

@andguy95 andguy95 commented Feb 25, 2026

WHY are these changes introduced?

When modifying skeleton template files, contributors have no automatic signal that cookbook recipes may also need to be updated. This can result in recipes silently drifting out of sync with the skeleton.

WHAT is this pull request doing?

Adds a pre-commit hook and two new cookbook CLI commands to help contributors keep recipes in sync with the skeleton template:

  • affected-recipes [files..]: given a list of repo-relative skeleton file paths, outputs the names of any cookbook recipes that reference those files (supports --json flag)
  • skeleton-files: lists all skeleton files referenced by recipes (or a specific recipe), with an --existing-only flag to filter to files that currently exist on disk
  • .husky/pre-commit hook: when a commit stages changes under templates/skeleton/, the hook runs affected-recipes against the staged files and prints a warning listing which recipes may need regenerating, along with the exact regenerate commands to run. The hook is informational only (always exits 0) and can be bypassed with --no-verify.

Supporting this, a dependency-graph library module was added to cookbook/src/lib/ to build the file→recipe mapping by reading all recipe YAML definitions.

HOW to test your changes?

  1. Stage a change to any file under templates/skeleton/
  2. Run git commit — you should see a warning listing affected recipes and regenerate commands
    • If you don't want to run git commit you can also test the affected recipes running bash cookbook/scripts/pre-commit.sh in the root of project
  3. Confirm the commit still succeeds (hook exits 0)

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@shopify
Copy link
Contributor

shopify bot commented Feb 25, 2026

Oxygen deployed a preview of your an-feat-skeleton-recipe-commit-hook branch. Details:

Storefront Status Preview link Deployment details Last update (UTC)
Skeleton (skeleton.hydrogen.shop) ✅ Successful (Logs) Preview deployment Inspect deployment February 27, 2026 9:01 PM

Learn more about Hydrogen's GitHub integration.

@fredericoo fredericoo changed the title feat: Adds in skeleton recipe pro-commit hook feat: Adds in skeleton recipe pre-commit hook Feb 25, 2026
Realizing that we need to exclude detection of skeleton changes to files
that will be deleted. Updated the script and the README as well.
@andguy95 andguy95 marked this pull request as ready for review February 25, 2026 21:17
@andguy95 andguy95 requested a review from a team as a code owner February 25, 2026 21:17
},
},
handler({ files, json }) {
const affected = getAffectedRecipes(files as string[]);
Copy link
Contributor

Choose a reason for hiding this comment

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

blocking: Once string: true is added to the builder (see comment above), this as string[] assertion can be removed. I want to remove as from this repo because of the type safety issues, so I am also trying not to add any new ones in :)

Suggested change
const affected = getAffectedRecipes(files as string[]);
const affected = getAffectedRecipes(files);

@andguy95 andguy95 requested a review from kdaviduik February 27, 2026 21:04
@binks-code-reviewer
Copy link

⚠️ Findings outside the diff

These findings are in files not modified by this PR and cannot be posted as inline comments.


/.husky/pre-commit:1Pre-commit hook chaining can block commits despite informational-only intent

The Husky hook is chained with &&:

npx lint-staged && npm run pre-commit:encrypt && bash cookbook/scripts/pre-commit.sh

This means the informational script will never run if lint-staged or pre-commit:encrypt fails. Additionally, if bash is unavailable (common on Windows without Git Bash), the hook fails at the shell level and blocks commits. This contradicts the stated intent that the hook is informational-only and always exits 0.

Impact: contributors can be prevented from committing locally, especially on Windows/minimal environments, increasing friction and encouraging --no-verify usage.

@binks-code-reviewer
Copy link

🤖 Code Review · #projects-dev-ai for questions
React with 👍/👎 or reply — all feedback helps improve the agent.

Complete - No issues

📋 History

❌ Failed → ✅ No issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants