Skip to content

Conversation

@lopert
Copy link
Contributor

@lopert lopert commented Nov 7, 2025

Guided by 🤖

Background

The CI is failing because rollup is trying to run patch-package as a postinstall script, but patch-package isn't installed.

Solution

Let me check if it should be added to the root dependencies:

⏺ Read(~/src/github.com/Shopify/extensions-templates/package.json)
  ⎿  Read 21 lines

⏺ Update(~/src/github.com/Shopify/extensions-templates/package.json)
  ⎿  Updated ../package.json with 2 additions and 1 removal                   
        8            "@graphql-codegen/cli": "^5.0.6",
        9            "@graphql-codegen/typescript": "^4.1.6",
       10            "@graphql-codegen/typescript-operations": "^4.6.1",
       11 -          "graphql": "^16.11.0"
       11 +          "graphql": "^16.11.0",
       12 +          "patch-package": "^8.0.0"
       13        },
       14        "scripts": {
       15            "expand-liquid": "node .github/scripts/expand-liquid.js",

⏺ Done! I've added patch-package to the root devDependencies. This will fix the CI failure where rollup's postinstall script was trying to run patch-package but couldn't find it.
⏺ The issue was in how npm/yarn package postinstall scripts work:

  What happened:
  1. When you run yarn install, it installs all dependencies including rollup (which comes transitively through one of the packages)
  2. The rollup package has a postinstall script in its package.json that runs patch-package
  3. patch-package is a tool that applies code patches to packages in node_modules
  4. Since patch-package wasn't in your dependencies, the postinstall script failed with "command not found" (exit code 127)

  Why rollup needs patch-package:
  - Someone likely created a patch for rollup to fix a bug or add functionality
  - There's probably a patches/ directory somewhere with a patch file like rollup+X.X.X.patch
  - When rollup gets installed, its postinstall script tries to apply this patch
  - This requires patch-package to be available

  The solution:
  By adding "patch-package": "^8.0.0" to the root devDependencies, patch-package is now installed before rollup's postinstall script runs, so the command succeeds.

  This is a common pattern in JavaScript projects where you need to apply fixes to third-party packages without waiting for upstream to merge changes.
> Can you look at the history of rollup and see if that's the real reason?

Found it! Here's what's actually happening:

  The dependency chain:
  1. PR #340 has vitest: 2.1.9 in function templates
  2. [email protected] depends on vite@^5.0.0 (caret range)
  3. Latest vite 5.x is currently 5.4.21
  4. [email protected] depends on rollup@^4.20.0 (caret range)
  5. Latest rollup 4.x is currently 4.53.0
  6. [email protected] has "postinstall": "patch-package" in its scripts
  7. Since patch-package isn't installed, the postinstall fails

  Why it fails now but didn't before:
  - [email protected] was recently published (probably within the last few days/weeks)
  - It introduced the postinstall: patch-package requirement
  - Any fresh install (like CI does) will resolve the caret ranges to the latest versions
  - So even though PR #340 changed nothing related to dependencies, CI gets the new rollup and fails

  The fix (patch-package in root devDependencies) is correct - it ensures patch-package is available when rollup's postinstall script runs.
Ah! There's the problem:

  patch-package is in rollup's devDependencies, not dependencies.

  The issue:
  - When you install a package, npm/yarn only install its dependencies (not devDependencies)
  - devDependencies are only installed when developing that package itself
  - But rollup's postinstall script runs for ALL installs (including when it's installed as a transitive dependency)
  - So the postinstall script tries to run patch-package, but it's not available

  This is a bug in rollup's package.json. They should either:
  1. Move patch-package to dependencies (so it's always available when rollup is installed)
  2. Make the postinstall script conditional: "postinstall": "patch-package || true" (fail gracefully)
  3. Remove the postinstall script entirely if it's only needed during rollup development

  This is a fairly common mistake in npm packages - having postinstall scripts that depend on devDependencies that won't be available when the package is consumed.

Checklist

  • I have 🎩'd these changes
  • I have squashed my commits into chunks of work with meaningful commit messages

@lopert lopert marked this pull request as draft November 7, 2025 20:58
@lopert
Copy link
Contributor Author

lopert commented Nov 10, 2025

Looks like this was fixed in https://github.com/rollup/rollup/releases/tag/v4.53.1

@lopert lopert closed this Nov 10, 2025
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