Skip to content

Always show the footer in dev, by keeping the task display running#2417

Merged
matteodepalo merged 3 commits into
mainfrom
always-show-footer-in-dev
Jul 13, 2023
Merged

Always show the footer in dev, by keeping the task display running#2417
matteodepalo merged 3 commits into
mainfrom
always-show-footer-in-dev

Conversation

@shauns

@shauns shauns commented Jul 12, 2023

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Ensures that we are consistent in showing the preview footer in dev

WHAT is this pull request doing?

Keeps the dev task list open, with its footer, even when the tasks have completed running.

How to test your changes?

Init a "none" app, and add a subscription ui extension. Run dev and observe that the footer is displayed and can be previewed with (p)

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
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

  • Themes: @shopify/theme-code-tools
  • UI extensions: @shopify/ui-extensions-cli
    • Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
  • Hydrogen: @shopify/hydrogen
  • Other: @shopify/cli-foundations

@github-actions

github-actions Bot commented Jul 12, 2023

Copy link
Copy Markdown
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
73.66% (+0.02% 🔼)
5532/7510
🟡 Branches
70.22% (+0.02% 🔼)
2657/3784
🟡 Functions
72.47% (+0.03% 🔼)
1424/1965
🟡 Lines
75.31% (+0.02% 🔼)
5259/6983

Test suite run success

1292 tests passing in 637 suites.

Report generated by 🧪jest coverage report action from 029b497

Comment thread packages/app/src/cli/services/dev/output.ts Outdated
Comment thread packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx Outdated
Comment thread packages/app/src/cli/services/dev/output.ts Outdated
Comment thread packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx Outdated
@matteodepalo matteodepalo marked this pull request as ready for review July 13, 2023 10:24
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

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/private/node/ui/components/ConcurrentOutput.d.ts
@@ -18,6 +18,7 @@ export interface ConcurrentOutputProps {
         shortcuts: Shortcut[];
         subTitle?: string;
     };
+    keepRunningAfterProcessesResolve?: boolean;
 }
 /**
  * Renders output from concurrent processes to the terminal.

@matteodepalo matteodepalo added this pull request to the merge queue Jul 13, 2023
Merged via the queue into main with commit c81281b Jul 13, 2023
@matteodepalo matteodepalo deleted the always-show-footer-in-dev branch July 13, 2023 11:36
@shopify-shipit shopify-shipit Bot temporarily deployed to nightly July 13, 2023 13:57 Inactive
@shopify-shipit shopify-shipit Bot temporarily deployed to production July 26, 2023 11:18 Inactive
MitchLillie added a commit that referenced this pull request Apr 16, 2026
When processBatch threw, the while loop exited and any events already
queued during the failing batch were orphaned — stuck in this.queue
until the next enqueue() call. This caused the dev session pipeline to
silently stop processing updates after a transient error (e.g. network
timeout, GraphQL error), matching the symptom in issue #2417 where
hot reloading stops after ~12-15 changes.

Fix: catch errors inside the while loop so the processor continues
draining queued items. Fatal errors should be thrown by the caller
(e.g. DevSession) to terminate the process, not by the batch processor.

Also fix bundleExtensionsAndUpload: non-auth ClientError responses
now return {status: 'unknown-error'} instead of throwing AbortError,
keeping the error in the normal result flow (logged, added to
failedEvents for retry on next change).
MitchLillie added a commit that referenced this pull request Apr 16, 2026
Tracing: every file change gets a monotonic [build:N] ID that traces
through the full pipeline in --verbose output. Logs at every stage:
onChange → handleWatcherEvents → buildExtensions → generateExtensionTypes
→ emit('all') → DevSession.onEvent → validateAppEvent → processEvents
→ bundleExtensionsAndUpload → result

Levers (env vars to isolate the permanent failure in #2417):

  DEV_SKIP_GENERATE_TYPES=1
    Skip generateExtensionTypes() after build. Tests if this call
    hangs and permanently blocks emit('all').

  DEV_SKIP_RESCAN_IMPORTS=1
    Skip rescanImports() which can restart the file watcher. Tests
    if a watcher restart during vite mid-write permanently loses
    the admin extension's watched files.

  DEV_SERIALIZE_ONCHANGE=1
    Serialize onChange handlers with a mutex. Tests if concurrent
    .then() chains corrupting shared state (this.app, bundle dir)
    cause the permanent break.

  DEV_UPLOAD_TIMEOUT_MS=15000
    Add a timeout to bundleExtensionsAndUpload. Tests if a hung
    GCS upload or API call permanently blocks the SerialBatchProcessor.
MitchLillie added a commit that referenced this pull request Apr 16, 2026
Toggles source code strings in a hosted app every N seconds to trigger
repeated vite rebuilds. Used alongside the diagnostic levers to isolate
the permanent failure in #2417.
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.

2 participants