-
Notifications
You must be signed in to change notification settings - Fork 149
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
Revert "Fetch notifications in background" #5234
Revert "Fetch notifications in background" #5234
Conversation
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/public/node/notifications-system.d.ts@@ -166,25 +166,11 @@ export type Notifications = zod.infer<typeof NotificationsSchema>;
*/
export declare function showNotificationsIfNeeded(currentSurfaces?: string[], environment?: NodeJS.ProcessEnv): Promise<void>;
/**
- * Get notifications list from cache, that is updated in the background from bin/fetch-notifications.json.
+ * Get notifications list from cache (refreshed every hour) or fetch it if not present.
*
* @returns A Notifications object.
*/
export declare function getNotifications(): Promise<Notifications>;
-/**
- * Fetch notifications from the CDN and chache them.
- *
- * @returns A string with the notifications.
- */
-export declare function fetchNotifications(): Promise<Notifications>;
-/**
- * Fetch notifications in background as a detached process.
- *
- * @param currentCommand - The current Shopify command being run.
- * @param argv - The arguments passed to the current process.
- * @param environment - Process environment variables.
- */
-export declare function fetchNotificationsInBackground(currentCommand: string, argv?: string[], environment?: NodeJS.ProcessEnv): void;
/**
* Filters notifications based on the version of the CLI.
*
packages/cli-kit/dist/public/node/system.d.ts@@ -12,7 +12,6 @@ export interface ExecOptions {
input?: string;
signal?: AbortSignal;
externalErrorHandler?: (error: unknown) => Promise<void>;
- background?: boolean;
}
/**
* Opens a URL in the user's default browser.
|
Coverage report
Show files with reduced coverage 🔻
Test suite run success2000 tests passing in 904 suites. Report generated by 🧪jest coverage report action from 05afd3f |
/snapit |
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Test the snapshot by intalling your package globally: pnpm i -g @shopify/cli@0.0.0-snapshot-20250120124423
|
It looks like this is causing a new terminal to pop up for a second on Windows, so I'm reverting until we find a solution.
I've tried by enabling the windowsHide flag, but it's not working. Apparently Node doesn't allow it on Windows with
detached = true
.Reverts #5020