Skip to content

Conversation

@shauns
Copy link
Contributor

@shauns shauns commented Jan 27, 2025

Closes https://github.com/Shopify/develop-app-storage/issues/18

WHY are these changes introduced?

Removes the custom data specification and improves handling of remote extension specifications with different UID strategies. As DCDD is an app.toml managed module, we need to be able to indicate that it's UID strategy is single

WHAT is this pull request doing?

  • Removes the custom data specification and its associated configuration
  • Updates the UID strategy handling based on uidIsClientProvided flag

How to test your changes?

  1. Run the test suite to verify the updated remote specification handling
  2. Tophat: make an app, connect to DL org (has module enabled), all the following should work.
  • Run dev, add [metaobjects.author.fields]\nbirthday="date" , no failure
  • Deploy same config, no failure
  • Do a link into a new file, see the above config preserved

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Contributor Author

shauns commented Jan 27, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
75.42% (+0.02% 🔼)
8960/11880
🟡 Branches
70.54% (-0.08% 🔻)
4358/6178
🟡 Functions
75.24% (+0.06% 🔼)
2349/3122
🟡 Lines
75.94% (-0.01% 🔻)
8465/11147
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / theme_files_delete.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / store-context.ts
100%
78.57% (-21.43% 🔻)
100% 100%
🔴
... / extension.ts
51.43% (-3.12% 🔻)
0% 60%
54.55% (-1.7% 🔻)
🟢
... / payload.ts
95.45%
75% (-13.89% 🔻)
100% 100%
🟢
... / previewable-extension.ts
85.71% (-3.17% 🔻)
100% 66.67%
83.33% (-4.17% 🔻)
🟢
... / setup-dev-processes.ts
95.56% (+0.1% 🔼)
78.57% (-3.57% 🔻)
90.91%
94.87% (+0.13% 🔼)
🟢
... / json-schema.ts
93.55% (-0.07% 🔻)
86.05% 100%
93.48% (-0.07% 🔻)
🔴
... / api.ts
53.97% (-3.84% 🔻)
39.77% (-3.98% 🔻)
53.49% (-5.05% 🔻)
54.75% (-4.4% 🔻)

Test suite run success

2015 tests passing in 905 suites.

Report generated by 🧪jest coverage report action from 7be6815

@shauns shauns changed the title Set UID strategy for dynamically generated modules Drive DCDD from server specifications Jan 27, 2025
@shauns shauns added the #gsd:37045 label Jan 27, 2025 — with Graphite App
@shauns shauns marked this pull request as ready for review January 27, 2025 11:54
@shauns shauns requested a review from a team as a code owner January 27, 2025 11:54
@github-actions
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.

@shauns shauns force-pushed the Drive_DCDD_from_server_specification branch from d730595 to b8f4b35 Compare January 27, 2025 12:00
@shauns shauns force-pushed the shauns/01-27-json_schema_parsing_strips_extra_properties branch from ef6c4b7 to ae8f1b5 Compare January 27, 2025 12:12
@shauns shauns force-pushed the Drive_DCDD_from_server_specification branch from b8f4b35 to 4ae7fdf Compare January 27, 2025 12:12
@shauns shauns force-pushed the shauns/01-27-json_schema_parsing_strips_extra_properties branch from ae8f1b5 to 13991f2 Compare January 27, 2025 12:30
@shauns shauns force-pushed the Drive_DCDD_from_server_specification branch from 4ae7fdf to 8966797 Compare January 27, 2025 12:31
@shauns shauns force-pushed the shauns/01-27-json_schema_parsing_strips_extra_properties branch from 13991f2 to 61faa92 Compare January 27, 2025 12:33
@shauns shauns force-pushed the Drive_DCDD_from_server_specification branch from 8966797 to 63ecde0 Compare January 27, 2025 12:33
@shauns shauns self-assigned this Jan 27, 2025
Copy link
Contributor

@isaacroldan isaacroldan left a comment

Choose a reason for hiding this comment

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

🎩 and 👌

@shauns shauns force-pushed the shauns/01-27-json_schema_parsing_strips_extra_properties branch from 61faa92 to d92ad26 Compare January 28, 2025 12:33
@shauns shauns force-pushed the Drive_DCDD_from_server_specification branch from 63ecde0 to 4057215 Compare January 28, 2025 12:33
@shauns shauns force-pushed the shauns/01-27-json_schema_parsing_strips_extra_properties branch from d92ad26 to 4007c75 Compare January 28, 2025 12:41
@shauns shauns force-pushed the Drive_DCDD_from_server_specification branch from 4057215 to 7be6815 Compare January 28, 2025 12:43
@github-actions
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

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/json-schema.d.ts
@@ -1,5 +1,6 @@
 import { ParseConfigurationResult } from './schema.js';
 import { ErrorObject, SchemaObject } from 'ajv';
+export type HandleInvalidAdditionalProperties = 'strip' | 'fail';
 type AjvError = ErrorObject<string, {
     [key: string]: unknown;
 }>;
@@ -19,10 +20,11 @@ export declare function normaliseJsonSchema(schema: string): Promise<SchemaObjec
  *
  * @param subject - The object to validate.
  * @param schema - The JSON schema to validate against.
+ * @param handleInvalidAdditionalProperties - Whether to strip or fail on invalid additional properties.
  * @param identifier - The identifier of the schema being validated, used to cache the validator.
  * @returns The result of the validation. If the state is 'error', the errors will be in a zod-like format.
  */
-export declare function jsonSchemaValidate(subject: object, schema: SchemaObject, identifier: string): ParseConfigurationResult<unknown> & {
+export declare function jsonSchemaValidate(subject: object, schema: SchemaObject, handleInvalidAdditionalProperties: HandleInvalidAdditionalProperties, identifier?: string): ParseConfigurationResult<unknown> & {
     rawErrors?: AjvError[];
 };
 export {};
\ No newline at end of file

Base automatically changed from shauns/01-27-json_schema_parsing_strips_extra_properties to main January 28, 2025 12:56
@shauns shauns added this pull request to the merge queue Jan 28, 2025
Merged via the queue into main with commit 04b045f Jan 28, 2025
1 check passed
@shauns shauns deleted the Drive_DCDD_from_server_specification branch January 28, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants