Skip to content

Add --standard-events-inspector flag to theme preview.#7822

Merged
isaacroldan merged 12 commits into
mainfrom
fd-standard-events
Jun 16, 2026
Merged

Add --standard-events-inspector flag to theme preview.#7822
isaacroldan merged 12 commits into
mainfrom
fd-standard-events

Conversation

@frandiox

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

We are releasing standard storefront events and actions.

WHAT is this pull request doing?

Add a new flag to inject UI during development that helps debug events and actions.

How to test your changes?

Run shopify theme dev --standard-events-inspector in Horizon v4. You should see a floating window like the following:

image

Post-release steps

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

frandiox and others added 9 commits March 21, 2026 20:41
Add the --standard-events flow to theme dev and wire the option through the dev server context.

Prepare assets/standard-events.d.ts in the background, refresh it when possible, create assets/global.d.ts when missing, and create or update assets/jsconfig.json so the new type files are wired in without clobbering unrelated config.

Rewrite standard-events runtime URLs to use the dev bundle in HTML and JS content, inject the events inspector at the start of the head element with defer, and cover proxy, local asset, startup, and error-page paths with tests.

Also make jsconfig wiring robust when existing configs exclude *.d.ts files by forcing the new definitions through the files list.

Co-authored-by: Codex <codex@openai.com>
Use the standard events dev bundle by default in theme dev, while making inspector injection and local type setup explicit opt-in features. Keep the type setup asynchronous so server startup is not blocked.

Co-authored-by: OpenAI Codex <codex@openai.com>
Only treat the inspector as already injected when an actual inspector script tag is present, and add a regression test for plain URL strings in inline scripts.

Co-authored-by: OpenAI Codex <codex@openai.com>
Remove the --standard-events-types flag and all associated logic
(d.ts download, global.d.ts creation, jsconfig.json wiring) from
theme dev. This feature was too intrusive for the dev command.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts:
#	packages/theme/src/cli/commands/theme/dev.ts
#	packages/theme/src/cli/services/dev.test.ts
#	packages/theme/src/cli/utilities/theme-environment/proxy.test.ts
#	packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts
Remove merge-era mock setup from the theme dev service test now that the Standard Events assertions use the existing dev command test harness. This keeps the test focused on the new option plumbing without duplicating unrelated setup.

Co-authored-by: OpenAI Codex <codex@openai.com>
@frandiox
frandiox requested review from a team as code owners June 16, 2026 05:14
@github-actions github-actions Bot added the Area: @shopify/theme @shopify/theme package issues label Jun 16, 2026
if (proxyResponse.status < 400) {
outputDebug(`Proxy status: ${proxyResponse.status}. Returning proxy response.`)

if ((proxyResponse.headers.get('content-type') ?? '').includes('text/html')) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeah, this seems too broad. Every text/html content type response would be affected here?

@bashu-shopify

bashu-shopify commented Jun 16, 2026

Copy link
Copy Markdown

just checking that you're aware: it looks like this PR also adds standardEventsDevBundle, which is separate from the Inspector. I'm fine with it personally (devs should have the standard events dev bundle when working on a theme), but the PR description doesn't reflect that.

@lucyxiang

Copy link
Copy Markdown
Contributor

:Tophat: with flag shows inspector

Screenshot 2026-06-16 at 11 00 51

Without flag doesn't
Screenshot 2026-06-16 at 11 01 33

@bashu-shopify bashu-shopify left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

apart from the if-condition in tryProxyRequest, this looks good.

@bashu-shopify
bashu-shopify self-requested a review June 16, 2026 15:04
lucyxiang and others added 2 commits June 16, 2026 11:08
Regenerate the oclif manifest and README to include the new
--standard-events-inspector flag, and remove the unused export on
standardEventsBaseUrl flagged by knip.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Gate the proxy-fallback patching on ctx.options.standardEventsInspector so
the normal theme dev proxy path returns raw HTML as before, avoiding an
unintended behavior change for responses unrelated to the inspector.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@isaacroldan isaacroldan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok from the dev-experience side

Comment thread packages/theme/src/cli/services/dev.ts Outdated
port,
open: options.open,
liveReload: options['live-reload'],
standardEventsDevBundle: options['standard-events-inspector'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non blocking but if DevBundle always follow flag behaviour, does it need to be an option here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i think we can leave it bc in the future I would want to turn it on independently

Comment thread packages/theme/src/cli/services/dev.ts Outdated
renderDevSetupProgress()
.then(serverStart)
.then(() => {
.then(async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's leftover from a refactor, removing

@lucyxiang
lucyxiang force-pushed the fd-standard-events branch from e479f7f to c9a689f Compare June 16, 2026 15:39

@lucyxiang lucyxiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, @frandiox one change I made was made standardEventsDevBundle true only when standardEventsInspector flag is passed. In the original PR standardEventsDevBundle was always true
Nvm intention decision from https://shopify.slack.com/archives/C099734SGN7/p1776188523891689?thread_ts=1776186844.346429&cid=C099734SGN7

- Wrap the proxy-fallback condition to satisfy prettier/max-len
- Regenerate shopify.dev docs so --standard-events-inspector is documented

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lucyxiang
lucyxiang force-pushed the fd-standard-events branch from c9a689f to babb7f4 Compare June 16, 2026 15:51

@bashu-shopify bashu-shopify left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm

@isaacroldan
isaacroldan merged commit b0994bb into main Jun 16, 2026
44 of 51 checks passed
@isaacroldan
isaacroldan deleted the fd-standard-events branch June 16, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: @shopify/theme @shopify/theme package issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants