Skip to content
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

Update mutation variables for AppVersionCreate #5451

Open
wants to merge 1 commit into
base: dlm-app-create-mutation-variables
Choose a base branch
from

Conversation

dmerand
Copy link
Contributor

@dmerand dmerand commented Feb 21, 2025

WHY are these changes introduced?

Following up to #5436 per this issue. Per these issues, we've updated the API shape on the back-end and need to update the CLI to match.

WHAT is this pull request doing?

This changes the CreateAppVersion GraphQL call to use the updated AppVersionInput data shape. I've also added an AppVersionSourceWithUrl interface to help to ensure that we're conforming to the API locally. See the previous PR for a brief discussion of that.

We also didn't have any tests for the deploy command, so I've added some.

How to test your changes?

  • p shopify app init
  • p shopify app deploy - which will exercise the non-source-url branch
  • p shopify app generate extension and pick e.g. Admin Action or another extension that uses assets.
  • p shopify app deploy - which will exercise the source-url branch

Post-release steps

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

Copy link
Contributor

github-actions bot commented Feb 21, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
75.84% (+0.13% 🔼)
9124/12031
🟡 Branches
71.06% (+0.15% 🔼)
4463/6281
🟡 Functions
75.52% (+0.13% 🔼)
2379/3150
🟡 Lines
76.36% (+0.11% 🔼)
8614/11281

Test suite run success

2069 tests passing in 923 suites.

Report generated by 🧪jest coverage report action from 8d20719

@dmerand dmerand force-pushed the dlm-app-version-create-mutation-variables branch from 89e0622 to f712bad Compare February 21, 2025 15:11
mutation CreateAppVersion($appId: ID!, $appSource: AppSourceInput!, $name: String!, $metadata: VersionMetadataInput) {
appVersionCreate(appId: $appId, appSource: $appSource, name: $name, metadata: $metadata) {
mutation CreateAppVersion($appId: ID!, $version: AppVersionInput!, $metadata: VersionMetadataInput) {
appVersionCreate(appId: $appId, version: $version, metadata: $metadata) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pulled name as well, on the theory that we're either going to pull name from the branding module (short-term), or update the manifest format to include it longer-term.

@dmerand dmerand force-pushed the dlm-app-version-create-mutation-variables branch from f712bad to 0c61cd7 Compare February 21, 2025 15:41
@dmerand dmerand marked this pull request as ready for review February 21, 2025 15:44
@dmerand dmerand requested a review from a team as a code owner February 21, 2025 15:44
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

@dmerand dmerand marked this pull request as draft February 21, 2025 15:51
@dmerand dmerand force-pushed the dlm-app-version-create-mutation-variables branch from 0c61cd7 to 8d20719 Compare February 21, 2025 16:57
Copy link
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

packages/cli-kit/dist/private/node/session/identity-token-validation.d.ts
export declare function validateIdentityToken(token: string): Promise<boolean>;

Existing type declarations

packages/cli-kit/dist/private/node/conf-store.d.ts
@@ -3,14 +3,16 @@ interface CacheValue<T> {
     value: T;
     timestamp: number;
 }
+export type IntrospectionUrlKey = ;
 export type PackageVersionKey = ;
 export type NotificationsKey = ;
 export type NotificationKey = ;
 export type GraphQLRequestKey = ;
 type MostRecentOccurrenceKey = ;
 type RateLimitKey = ;
-type ExportedKey = PackageVersionKey | NotificationsKey | NotificationKey | GraphQLRequestKey;
+type ExportedKey = IntrospectionUrlKey | PackageVersionKey | NotificationsKey | NotificationKey | GraphQLRequestKey;
 interface Cache {
+    [introspectionUrlKey: IntrospectionUrlKey]: CacheValue<string>;
     [packageVersionKey: PackageVersionKey]: CacheValue<string>;
     [notifications: NotificationsKey]: CacheValue<string>;
     [notification: NotificationKey]: CacheValue<string>;
@@ -57,13 +59,12 @@ export declare function cacheStore(key: ExportedKey, value: string, config?: Loc
  */
 export declare function cacheRetrieve(key: ExportedKey, config?: LocalStorage<ConfSchema>): CacheValue<string> | undefined;
 export declare function cacheClear(config?: LocalStorage<ConfSchema>): void;
-export interface TimeInterval {
+interface TimeInterval {
     days?: number;
     hours?: number;
     minutes?: number;
     seconds?: number;
 }
-export declare function timeIntervalToMilliseconds({ days, hours, minutes, seconds }: TimeInterval): number;
 /**
  * Execute a task only if the most recent occurrence of the task is older than the specified timeout.
  * @param key - The key to use for the cache.
packages/cli-kit/dist/public/node/api/graphql.d.ts
@@ -1,4 +1,4 @@
-import { ConfSchema, TimeInterval } from '../../../private/node/conf-store.js';
+import { ConfSchema } from '../../../private/node/conf-store.js';
 import { LocalStorage } from '../local-storage.js';
 import { rawRequest, RequestDocument, Variables } from 'graphql-request';
 import { TypedDocumentNode } from '@graphql-typed-document-node/core';
@@ -12,10 +12,11 @@ export interface GraphQLVariables {
 }
 export type GraphQLResponse<T> = Awaited<ReturnType<typeof rawRequest<T>>>;
 export interface CacheOptions {
-    cacheTTL: TimeInterval;
+    cacheTTL: CacheTTL;
     cacheExtraKey?: string;
     cacheStore?: LocalStorage<ConfSchema>;
 }
+export type CacheTTL = number;
 interface GraphQLRequestBaseOptions<TResult> {
     api: string;
     url: string;

@dmerand dmerand marked this pull request as ready for review February 21, 2025 17:02
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.

1 participant