-
Notifications
You must be signed in to change notification settings - Fork 242
Add appAssets to the websocket app payload #7226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: graphite-base/7226
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,11 +9,17 @@ import { | |
| import {AppEvent, AppEventWatcher, EventType} from './app-events/app-event-watcher.js' | ||
| import {buildCartURLIfNeeded} from './extension/utilities.js' | ||
| import {ExtensionInstance} from '../../models/extensions/extension-instance.js' | ||
| import {globPatternBaseDir} from '../../models/extensions/specification.js' | ||
| import {AbortSignal} from '@shopify/cli-kit/node/abort' | ||
| import {outputDebug} from '@shopify/cli-kit/node/output' | ||
| import {normalizePath} from '@shopify/cli-kit/node/path' | ||
| import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' | ||
| import {Writable} from 'stream' | ||
|
|
||
| interface AppAssets { | ||
| [key: string]: string | ||
| } | ||
|
|
||
| export interface ExtensionDevOptions { | ||
| /** | ||
| * Standard output stream to send the output through. | ||
|
|
@@ -112,6 +118,28 @@ export interface ExtensionDevOptions { | |
| * The app watcher that emits events when the app is updated | ||
| */ | ||
| appWatcher: AppEventWatcher | ||
|
|
||
| /** | ||
| * Map of asset key to absolute directory path for app-level assets (e.g., admin static_root) | ||
| */ | ||
| appAssets?: AppAssets | ||
| } | ||
|
|
||
| /** | ||
| * Derives app-level asset directories from config extensions that define devSessionWatchConfig | ||
| * with an assetKey. Returns a map of asset key (e.g. 'static_root') to absolute directory path. | ||
| */ | ||
| export function resolveAppAssets(allExtensions: ExtensionInstance[]): Record<string, string> { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can switch this back to using the |
||
| const appAssets: Record<string, string> = {} | ||
| for (const ext of allExtensions) { | ||
| if (!ext.isAppConfigExtension) continue | ||
| const watchConfig = ext.devSessionWatchConfig | ||
| if (!watchConfig || watchConfig.paths.length === 0 || !watchConfig.assetKey) continue | ||
|
|
||
| const baseDir = normalizePath(globPatternBaseDir(watchConfig.paths[0]!)) | ||
| appAssets[watchConfig.assetKey] = baseDir | ||
| } | ||
| return appAssets | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This solution is very hacky. We are adding the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This solution is very hacky. We are adding the |
||
| } | ||
|
|
||
| export async function devUIExtensions(options: ExtensionDevOptions): Promise<void> { | ||
|
|
@@ -133,17 +161,29 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi | |
| } | ||
|
|
||
| outputDebug(`Setting up the UI extensions HTTP server...`, payloadOptions.stdout) | ||
| const httpServer = setupHTTPServer({devOptions: payloadOptions, payloadStore, getExtensions}) | ||
| const getAppAssets = () => payloadOptions.appAssets | ||
| const httpServer = setupHTTPServer({ | ||
| devOptions: payloadOptions, | ||
| payloadStore, | ||
| getExtensions, | ||
| getAppAssets, | ||
| }) | ||
|
|
||
| outputDebug(`Setting up the UI extensions Websocket server...`, payloadOptions.stdout) | ||
| const websocketConnection = setupWebsocketConnection({...payloadOptions, httpServer, payloadStore}) | ||
| outputDebug(`Setting up the UI extensions bundler and file watching...`, payloadOptions.stdout) | ||
|
|
||
| const eventHandler = async ({appWasReloaded, app, extensionEvents}: AppEvent) => { | ||
| const eventHandler = async ({appWasReloaded, app, extensionEvents, appAssetsUpdated}: AppEvent) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you also have the app here from the AppEvent, this one is better because is the updated app (in case anything changed) |
||
| if (appWasReloaded) { | ||
| extensions = app.allExtensions.filter((ext) => ext.isPreviewable) | ||
| } | ||
|
|
||
| if (appAssetsUpdated && payloadOptions.appAssets) { | ||
| for (const assetKey of Object.keys(payloadOptions.appAssets)) { | ||
| payloadStore.updateAppAssetTimestamp(assetKey) | ||
| } | ||
| } | ||
|
|
||
| for (const event of extensionEvents) { | ||
| if (!event.extension.isPreviewable) continue | ||
| const status = event.buildResult?.status === 'ok' ? 'success' : 'error' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added so that we can use this config in the
resolveAppAssets()We don't have to do this if we're okay with directly using
Appobject to find theappAssetdirectory