-
Notifications
You must be signed in to change notification settings - Fork 224
Group events by extension to reduce triggered builds #6558
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3358 tests passing in 1372 suites. Report generated by 🧪jest coverage report action from 268feef |
8e08b52 to
d7ebe8d
Compare
53e1e4f to
a515640
Compare
d7ebe8d to
df86305
Compare
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
packages/app/src/cli/services/dev/app-events/app-event-watcher.ts
Outdated
Show resolved
Hide resolved
df86305 to
3326b7e
Compare
a515640 to
2c0f953
Compare
3326b7e to
4e68502
Compare
gonzaloriestra
left a comment
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.
It's not working for me. I'm trying with an admin-print extension, making two changes to the en and fr locales and using Save all in the editor to save both at the same time, and I'm getting this output:
14:29:46 │ admin-print │ Build successful
14:29:46 │ admin-print │ Extension changed
14:29:46 │ admin-print │ Build successful
14:29:49 │ app-preview │ ✅ Updated app preview on gonzaloriestra-personal-dev-dd.myshopify.com
14:29:49 │ admin-print │ Extension changed
14:29:51 │ app-preview │ ✅ Updated app preview on gonzaloriestra-personal-dev-dd.myshopify.com
They shouldn't appear duplicated, right?
|
The changes have to be within the debounce time of the file watcher to be grouped, that is in less than 200ms aprox. It may be that you are not doing that fast enough to see them grouped? |
|
The first change is not debounced, the CLI starts debouncing after the second change. This is to optimize for the most common case: a single change. If we were to debounce that, we would be adding an extra 200ms every time. |
|
We could have a smaller debouncer for the first change only, like 20-30ms to catch automatic saves that modify multiple files at once 🤔 |
2c0f953 to
aec5821
Compare
4e68502 to
5e2a2ce
Compare
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/private/node/analytics.d.ts@@ -27,6 +27,5 @@ interface EnvironmentData {
export declare function getEnvironmentData(config: Interfaces.Config): Promise<EnvironmentData>;
export declare function getSensitiveEnvironmentData(config: Interfaces.Config): Promise<{
env_plugin_installed_all: string;
- env_shopify_variables: string;
}>;
export {};
\ No newline at end of file
packages/cli-kit/dist/private/node/api.d.ts@@ -11,30 +11,6 @@ type RequestOptions<T> = {
request: () => Promise<T>;
url: string;
} & NetworkRetryBehaviour;
-/**
- * Checks if an error is a transient network error that is likely to recover with retries.
- *
- * Use this function for retry logic. Use isNetworkError for error classification.
- *
- * Examples of transient errors (worth retrying):
- * - Connection timeouts, resets, and aborts
- * - DNS failures (enotfound, getaddrinfo, eai_again) - can be temporary
- * - Socket disconnects and hang ups
- * - Premature connection closes
- */
-export declare function isTransientNetworkError(error: unknown): boolean;
-/**
- * Checks if an error is any kind of network-related error (connection issues, timeouts, DNS failures,
- * TLS/certificate errors, etc.) rather than an application logic error.
- *
- * These errors should be reported as user-facing errors (AbortError) rather than bugs (BugError),
- * regardless of whether they are transient or permanent.
- *
- * Examples include:
- * - Transient: connection timeouts, socket hang ups, temporary DNS failures
- * - Permanent: certificate validation failures, misconfigured SSL
- */
-export declare function isNetworkError(error: unknown): boolean;
export declare function simpleRequestWithDebugLog<T extends {
headers: Headers;
status: number;
packages/cli-kit/dist/public/node/metadata.d.ts@@ -55,7 +55,6 @@ declare const coreData: RuntimeMetadataManager<CmdFieldsFromMonorail, {
cmd_all_environment_flags?: string | null;
cmd_dev_tunnel_custom?: string | null;
env_plugin_installed_all?: string | null;
- env_shopify_variables?: string | null;
}, "store_", never>>;
export declare const getAllPublicMetadata: () => Partial<CmdFieldsFromMonorail>, getAllSensitiveMetadata: () => Partial<{
commandStartOptions: {
@@ -75,7 +74,6 @@ export declare const getAllPublicMetadata: () => Partial<CmdFieldsFromMonorail>,
cmd_all_environment_flags?: string | null;
cmd_dev_tunnel_custom?: string | null;
env_plugin_installed_all?: string | null;
- env_shopify_variables?: string | null;
}, "store_", never>>, addPublicMetadata: (getData: ProvideMetadata<CmdFieldsFromMonorail>, onError?: MetadataErrorHandling) => Promise<void>, addSensitiveMetadata: (getData: ProvideMetadata<{
commandStartOptions: {
startTime: number;
@@ -94,7 +92,6 @@ export declare const getAllPublicMetadata: () => Partial<CmdFieldsFromMonorail>,
cmd_all_environment_flags?: string | null;
cmd_dev_tunnel_custom?: string | null;
env_plugin_installed_all?: string | null;
- env_shopify_variables?: string | null;
}, "store_", never>>, onError?: MetadataErrorHandling) => Promise<void>, runWithTimer: (field: NumericKeyOf<CmdFieldsFromMonorail>) => <T>(fn: () => Promise<T>) => Promise<T>;
export type Public = PublicSchema<typeof coreData>;
export type Sensitive = SensitiveSchema<typeof coreData>;
packages/cli-kit/dist/public/node/monorail.d.ts@@ -2,7 +2,7 @@ import { JsonMap } from '../../private/common/json.js';
import { DeepRequired } from '../common/ts/deep-required.js';
export { DeepRequired };
type Optional<T> = T | null;
-export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.20";
+export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.19";
export interface Schemas {
[MONORAIL_COMMAND_TOPIC]: {
sensitive: {
@@ -14,7 +14,6 @@ export interface Schemas {
cmd_all_environment_flags?: Optional<string>;
cmd_dev_tunnel_custom?: Optional<string>;
env_plugin_installed_all?: Optional<string>;
- env_shopify_variables?: Optional<string>;
};
public: {
business_platform_id?: Optional<number>;
packages/cli-kit/dist/cli/api/graphql/admin/generated/theme_files_upsert.d.ts@@ -8,7 +8,6 @@ export type ThemeFilesUpsertMutation = {
themeFilesUpsert?: {
upsertedThemeFiles?: {
filename: string;
- checksumMd5?: string | null;
}[] | null;
userErrors: {
filename?: string | null;
|
aec5821 to
bb52e42
Compare
5e2a2ce to
d724888
Compare
bb52e42 to
d6c771d
Compare
d724888 to
268feef
Compare

WHY are these changes introduced?
The current implementation of
buildExtensionsprocesses each extension event individually, which can lead to inefficient handling when multiple events occur for the same extension.WHAT is this pull request doing?
Optimizes the extension build process by grouping extension events by extension UID before processing them. This ensures that when multiple events occur for the same extension, we only trigger one build operation instead of multiple redundant builds.
Key changes:
How to test your changes?
devwith multiple extensionsMeasuring impact
How do we know this change was effective? Please choose one:
Checklist