Fix custom-oclif-loader.ts hydrogen loader strategy for hydrogen-cli local development by andguy95 · Pull Request #6827 · Shopify/cli

andguy95 · 2026-02-05T20:59:14Z

WHY are these changes introduced?

During local development of the @shopify/cli in the Shopify/hydrogen you are no longer able to see your changes when running the npx shopify hydrogen commands because the custom-ocliff-loader.ts no longer loads the local Shopify/hydrgen cli in the Shopify/hydrogen monorepo.

These versions below are good:

3.80.6 ✅
3.80.7 ✅
3.81.0 ✅
3.81.1 ✅
3.81.2 ✅
3.82.0 ✅
3.82.1 ✅
3.83.0 ❌ - Looks like the hydrogen mono repo awareness stopped working here (looks like major bump of OCLIF)

In OCLIF v4, determinePriority is a standalone utility function (not an instance method), so we can't override it to customize command priority in custom-ocliff-loader.ts.

WHAT is this pull request doing?

Change the custom-ocliff-loader.ts to manually replace bundled hydrogen commands with external ones after loading completes if in dev mode.

How to test your changes?

Go to dev.ts in hydrogen/packages/cli/src/commands/hydrogen/dev.ts
Make arbitrary update to the run()
In the cli project run npm run build
In the template/skeleton project run npx shopify hydrogen dev
- To test locally i created an alias shopify-dev pointing to a locally built version of the Shopiy/cli

# Create the following file in ~/bin/shopify:#!/usr/bin/env node

process.env.SHOPIFY_RUBY_BINDIR = "/opt/rubies/3.1.2/bin"
process.env.SHOPIFY_HOMEBREW_FORMULA = "shopify-cli"

import("/Users/<your-user>/src/github.com/Shopify/cli/packages/cli/bin/run.js")

alias shopify-dev='/Users/$USER/bin/shopify'

Measuring impact

How do we know this change was effective? Please choose one:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

…l dev

github-actions · 2026-02-05T21:05:56Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	78.91%	14518/18398
🟡	Branches	73.44%	7225/9838
🟡	Functions	79.05%	3690/4668
🟡	Lines	79.25%	13723/17315

Test suite run success

3789 tests passing in 1453 suites.

Report generated by 🧪jest coverage report action from 72d3411

EvilGenius13 · 2026-02-05T21:46:37Z

Changes make sense, thank you for the pairing @andguy95. We'll want to make sure @Shopify/app-inner-loop takes a peek as they have historically made the changes to the file.

packages/cli-kit/src/public/node/custom-oclif-loader.ts

kdaviduik · 2026-02-10T07:12:48Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

 import {isDevelopment} from './context/local.js'
 import {execaSync} from 'execa'
-import {Command, Config} from '@oclif/core'
+import {Config} from '@oclif/core'


(threading)

I think we should add tests here too. Otherwise we aren't fixing the true root cause, which is regressions in this file can break this functionality for the Hydrogen CLI, and no tests or CI checks will fail as a result.

I think we should add at minimum:

A test verifying external hydrogen commands replace bundled ones after load()

A test verifying non-dev mode does NOT trigger replacement

A test that would catch future oclif API changes

kdaviduik · 2026-02-10T07:13:46Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

 import {isDevelopment} from './context/local.js'
 import {execaSync} from 'execa'
-import {Command, Config} from '@oclif/core'
+import {Config} from '@oclif/core'


(threading)

The PR only replaces commands by command.id. If hydrogen commands have aliases registered in _commands, those alias entries still point to bundled versions. There are currently no hydrogen command aliases (checked all 26 commands in oclif.manifest.json), so this is not a bug today but it's a latent issue.

I would recommend adding a comment documenting this limitation, or handling aliases proactively, but there might be a better way to do this overall that avoids this issue:

oclif's own insertLegacyPlugins method uses a delete-then-loadCommands pattern. I think using this.loadCommands(externalHydrogenPlugin) after deleting bundled commands would be a bit cleaner since it handles aliases and permutations through oclif's own code, though both options would involve the ts-expect-error from accessing internal properties

Updated the logic to match the delete and load pattern found in the insert legacy plugin. This should also account for the aliasing!

packages/cli-kit/src/public/node/custom-oclif-loader.ts

andguy95 · 2026-02-19T22:54:32Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

+    for (const command of externalHydrogenPlugin.commands) {
+      if (!command.id.startsWith('hydrogen')) continue
+      internalCommands.delete(command.id)
+      const allAliases = [...(command.aliases ?? []), ...((command as {hiddenAliases?: string[]}).hiddenAliases ?? [])]


Here is the code where we delete the aliased commands so we don't have a stale/cached version if aliased

andguy95 · 2026-02-19T22:57:19Z

packages/cli-kit/src/public/node/custom-oclif-loader.test.ts

+  // if oclif removes or renames the private APIs that ShopifyConfig depends on.
+  // The mock above intentionally replaces oclif for logic isolation; this block
+  // provides the missing contract check against the installed package version.
+  describe('oclif API contract', () => {


Added test to check for possible regression:

loadCommands needs to be on the config

_commands needs to be on the config

This will also be caught in the error check in the actual loader

andguy95 · 2026-02-19T22:58:33Z

packages/cli-kit/src/public/node/custom-oclif-loader.test.ts

+  })
+
+  describe('load()', () => {
+    test('does not replace commands when not in dev mode', async () => {


A test verifying non-dev mode does NOT trigger replacement

andguy95 · 2026-02-19T23:03:28Z

packages/cli-kit/src/public/node/custom-oclif-loader.test.ts

+      expect(asMock(config).loadCommandsCalls).toHaveLength(0)
+    })
+
+    test('removes bundled hydrogen commands and reloads from external plugin', async () => {


A test verifying external hydrogen commands replace bundled ones after load()

kdaviduik

Needs a changeset but other than that LGTM :)

note about this, it seems to be a false alarm as apparently:

customPriority had zero callers anywhere in the codebase
customPriority was already non-functional since oclif v3.83.0
The sole consumer (cli-launcher.ts) already calls config.load() — no caller changes needed

github-actions · 2026-02-26T22:21:51Z

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/custom-oclif-loader.d.ts

@@ -1,6 +1,6 @@
-import { Command, Config } from '@oclif/core';
+import { Config } from '@oclif/core';
 import { Options } from '@oclif/core/interfaces';
 export declare class ShopifyConfig extends Config {
     constructor(options: Options);
-    customPriority(commands: Command.Loadable[]): Command.Loadable | undefined;
+    load(): Promise<void>;
 }
\ No newline at end of file

binks-code-reviewer · 2026-02-26T22:28:29Z

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

✅ Complete - No issues

📋 History

✅ No issues

kdaviduik · 2026-02-27T04:58:48Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

+    for (const command of externalHydrogenPlugin.commands) {
+      if (!command.id.startsWith('hydrogen')) continue
+      internalCommands.delete(command.id)
+      const allAliases = [...(command.aliases ?? []), ...((command as {hiddenAliases?: string[]}).hiddenAliases ?? [])]


non-blocking: hiddenAliases is an unguarded third private API dependency

The type assertion to {hiddenAliases?: string[]} accesses an undocumented oclif property. The contract tests verify _commands and loadCommands on the real Config but do NOT verify hiddenAliases on command objects. If oclif renames or removes hiddenAliases, the ?? [] fallback silently produces an empty array and hidden aliases for bundled commands remain in _commands, causing the bundled version to win when invoked via a hidden alias.

I think adding a third contract test would close this gap:

test('Command objects still expose hiddenAliases', async () => { const {Config: RealConfig} = await vi.importActual<typeof import('@oclif/core')>('@oclif/core') const config = new (RealConfig as new (options: {root: string}) => OclifConfig)({ root: fileURLToPath(new URL('.', import.meta.url)), }) await config.load() const someCommand = Array.from(config.commands)[0] if (someCommand) { expect('hiddenAliases' in someCommand).toBe(true) } })

I'd also suggest extracting the inline type cast to a named interface for readability:

interface OclifCommand { id: string aliases?: string[] hiddenAliases?: string[] }

kdaviduik · 2026-02-27T04:58:48Z

.changeset/pretty-swans-smell.md

+'@shopify/cli-kit': patch
+---
+
+Fix hydrogen local dev plugin loader to correctly replace bundled hydrogen commands with external plugin commands by switching to a post-load strategy


blocking: @shopify/cli-kit exports custom-oclif-loader.js as a public entry point. The type diff shows customPriority was removed from the public declaration and load() was added. While customPriority was effectively dead code in oclif v4 (the framework no longer called it), it was part of the public type declaration.

The changeset is marked as patch, which I think is appropriate if there are no external consumers, but I think the changeset body should also mention the customPriority removal for transparency, even if it's dead code - consumers upgrading @shopify/cli-kit should be aware.

kdaviduik · 2026-02-27T04:58:48Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

-        return -1
-      }
+    if (typeof (this as unknown as {[key: string]: unknown})._commands === 'undefined') {
+      throw new Error('ShopifyConfig: oclif internals changed. _commands is no longer available.')


non-blocking: I think we should also check instanceof Map here

The typeof check catches _commands being removed, but if oclif changes _commands from Map to a plain object or other collection type, the code would crash at .delete(). An instanceof Map check should throw (same pattern as this guard) rather than silently skipping:

// After the typeof check, add: const internalCommands = this._commands if (!(internalCommands instanceof Map)) { throw new Error('ShopifyConfig: oclif internals changed. _commands is no longer a Map.') }

kdaviduik · 2026-02-27T04:58:49Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

+        // Force OCLIF to ignore manifests so commands are loaded dynamically
+        // to be replaced later
+        options.ignoreManifest = true


non-blocking: I think this comment could be more precise about the tradeoff - this applies to ALL plugins, not just hydrogen:

Suggested change

// Force OCLIF to ignore manifests so commands are loaded dynamically

// to be replaced later

options.ignoreManifest = true

// Skip the oclif manifest cache so external plugin commands are loaded

// from disk rather than from a potentially stale manifest. This has a

// startup performance cost, but only applies during development.

options.ignoreManifest = true

isaacroldan · 2026-02-27T12:46:19Z

packages/cli-kit/src/public/node/custom-oclif-loader.ts

        }
+        // Force OCLIF to ignore manifests so commands are loaded dynamically
+        // to be replaced later
+        options.ignoreManifest = true


This is dangerous change, ignoring the manifest will make the CLI slower at startup on every command, because it needs to discover every command again on each run. Please do a comparison to check how speed is affected with and without this 🙏.

When is this line code executed? will it affect most users or just internal hydrogen development?

@isaacroldan Thank you, this is a good callout. I will test. This ideally should be executed in development mode but only when in the Hydrogen Monorepo.

In this instance we could probably move the hydrogen monorepo check where the isDevelopment check is to ensure this ignoreManifest only runs in the expected context.

isaacroldan · 2026-02-27T12:50:23Z

Is bringing hydrogen-cli back to this repo a possible alternative to this change? We are doing hacks to support a weird repo structure :/

isaacroldan · 2026-02-27T12:50:53Z

/snapit

github-actions · 2026-02-27T12:53:46Z

🫰✨ Thanks @isaacroldan! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260227125139

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

fix: updates to new oclif v4 plugin loader strategy for hydrogen loca…

9679c32

…l dev

andguy95 requested a review from a team as a code owner February 5, 2026 20:59

This comment has been minimized.

Sign in to view

EvilGenius13 approved these changes Feb 5, 2026

View reviewed changes

fix: variable length lint error

2b31382

kdaviduik reviewed Feb 10, 2026

View reviewed changes

fredericoo mentioned this pull request Feb 10, 2026

feat(ci): add scaffolding tests for npm, pnpm, and yarn Shopify/hydrogen#3458

Draft

4 tasks

andguy95 added 3 commits February 19, 2026 17:13

chore: address comments and adds tests

511a2de

fix: lint errors

5a5fc4b

fix: eslint type error

122dca8

andguy95 commented Feb 19, 2026

View reviewed changes

andguy95 requested review from EvilGenius13, amcaplan, craigmichaelmartin, fredericoo, gonzaloriestra, itsjustriley and kdaviduik February 19, 2026 23:03

fredericoo changed the title ~~Fix custom-ocliff-loader.ts hydrogen loader strategy for hydrogen-cli local development~~ Fix custom-oclif\-loader.ts hydrogen loader strategy for hydrogen-cli local development Feb 23, 2026

fredericoo changed the title ~~Fix custom-oclif\-loader.ts hydrogen loader strategy for hydrogen-cli local development~~ Fix custom-oclif-loader.ts hydrogen loader strategy for hydrogen-cli local development Feb 23, 2026

kdaviduik reviewed Feb 23, 2026

View reviewed changes

chore: adds a patch changeset for cli-kit

d47821b

andguy95 requested a review from a team as a code owner February 24, 2026 17:11

Merge branch 'main' into an-fix-hydrogen-cli-custom-loader

72d3411

kdaviduik reviewed Feb 27, 2026

View reviewed changes

isaacroldan reviewed Feb 27, 2026

View reviewed changes

Conversation

andguy95 commented Feb 5, 2026

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Measuring impact

Checklist

Uh oh!

This comment has been minimized.

github-actions bot commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report

Test suite run success

Uh oh!

EvilGenius13 commented Feb 5, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kdaviduik left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 26, 2026

Differences in type declarations

New type declarations

Existing type declarations

Uh oh!

binks-code-reviewer bot commented Feb 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isaacroldan commented Feb 27, 2026

Uh oh!

isaacroldan commented Feb 27, 2026

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Feb 5, 2026 •

edited

Loading