Skip to content

Watch assets for hot reloading#7437

Open
elanalynn wants to merge 1 commit intomainfrom
04-30-watch_assets_for_hot_reloading
Open

Watch assets for hot reloading#7437
elanalynn wants to merge 1 commit intomainfrom
04-30-watch_assets_for_hot_reloading

Conversation

@elanalynn
Copy link
Copy Markdown
Contributor

@elanalynn elanalynn commented Apr 30, 2026

WHY are these changes introduced?

Related to https://app.graphite.com/github/pr/shop/world/666583
Issue https://github.com/shop/issues-admin-extensibility/issues/2461

WHAT is this pull request doing?

I opened https://app.graphite.com/github/pr/shop/world/666583 in web to react to updated timestamps on assets served from the dev server, but we were still not watching new files. This PR

How to test your changes?

Demo of testing two images with name changes:
https://share.descript.com/view/bV9Pt8wvK6F

  • Check out this branch in CLI: 04-30-watch_assets_for_hot_reloading
  • Check out 04-29-preserve_timestamp_on_asset_src_and_bust_cache_when_asset_changes in admin-web
  • Create an app home project or clone ui-ext-hosted-jam (in ShopifyPlayground)
  • Create an assets directory in the app-home extension and add a couple images

Screenshot 2026-04-30 at 4.21.08 PM.png

  • Update the extension toml with the assets config
[[extensions.targeting]]
module = "./src/Home.jsx"
target = "admin.app.home.render"
assets = "./assets"
  • Add a couple images to a page like this
<s-grid gridTemplateColumns="1fr 1fr" gap="base">
  <s-image src="assets/dog.jpg" alt="dog" />
  <s-image src="assets/squirrel.jpg" alt="squirrel" />
</s-grid>
  • change the image names and confirm that the page reloads on every change (see video)

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

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Apr 30, 2026
@elanalynn elanalynn force-pushed the 04-30-watch_assets_for_hot_reloading branch from a100399 to 2246893 Compare April 30, 2026 22:15
@elanalynn elanalynn marked this pull request as ready for review April 30, 2026 22:22
@elanalynn elanalynn requested a review from a team as a code owner April 30, 2026 22:22
Copilot AI review requested due to automatic review settings April 30, 2026 22:22
@elanalynn elanalynn requested review from alfonso-noriega, byrichardpowell, melissaluu and vividviolet and removed request for Copilot April 30, 2026 22:24
// New files won't be in the watched-files snapshot yet, so fall back to
// matching the path against each extension's directory. `shouldIgnoreEvent`
// still applies gitignore and per-extension patterns afterwards.
const owningExtension = this.app.realExtensions.find((ext) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't work for extensions sharing assets. Maybe we could inspect the extension's config and see if the file path matches anything that was configured for the extension?

@melissaluu melissaluu force-pushed the 04-30-watch_assets_for_hot_reloading branch from 2246893 to bae6a0f Compare May 1, 2026 19:17
Copilot AI review requested due to automatic review settings May 1, 2026 19:17
@melissaluu melissaluu force-pushed the 04-30-watch_assets_for_hot_reloading branch from bae6a0f to 256ec29 Compare May 1, 2026 19:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the dev file-watcher so newly created files (especially assets) can be attributed to the correct extension even when they weren’t present in the initial “watched files” snapshot, enabling more reliable hot reloading behavior.

Changes:

  • Add a fallback path-to-extension ownership resolution when an event arrives for a file not present in the initial extensionWatchedFiles snapshot.
  • Attempt ownership resolution by checking configured extension_points[].assets directories first, then extension directory containment.
  • Add tests covering the new “unknown file” directory-containment fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/app/src/cli/services/dev/app-events/file-watcher.ts Adds fallback extension ownership detection for newly created/unknown files (assets dirs + extension dir containment).
packages/app/src/cli/services/dev/app-events/file-watcher.test.ts Adds tests validating the new fallback behavior for new files under an extension dir and ignored paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +264 to +268
const extensionPoints = (ext.configuration as {extension_points?: {assets?: string}[]})?.extension_points
if (extensionPoints) {
const matchesAssetDir = extensionPoints.some((ep) => {
if (!ep.assets) return false
const assetDir = normalizePath(joinPath(ext.directory, ep.assets))
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.

@vividviolet what do you think about this? It seems like we actually don't support watching external folders outside of the extension folder ATM so a shared asset folder won't actually be watched.

Comment on lines +281 to +286
for (const owningExtension of owningExtensions) {
this.handleEventForExtension(
event,
path,
normalizePath(owningExtension.directory),
startTime,
Comment on lines +848 to +852
describe('new file directory containment fallback', () => {
test('new file under an existing extension dir triggers file_created with the correct extensionHandle', async () => {
// Given: extension started with no files in assets/ — the new path is not in the snapshot
const ext = await testUIExtension({
type: 'ui_extension',
@melissaluu melissaluu force-pushed the 04-30-watch_assets_for_hot_reloading branch from 256ec29 to 40fc5df Compare May 1, 2026 21:03
@melissaluu melissaluu self-assigned this May 1, 2026
@melissaluu melissaluu force-pushed the 04-30-watch_assets_for_hot_reloading branch from 40fc5df to a36330a Compare May 1, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants