Read single-subscription events modules back into the TOML subscription list - #8357
Draft
dpeacock wants to merge 1 commit into
Draft
Read single-subscription events modules back into the TOML subscription list#8357dpeacock wants to merge 1 commit into
dpeacock wants to merge 1 commit into
Conversation
…on list Core is moving events modules to a one-module-per-subscription structure: each module's config carries events.subscription as a single object instead of a list. transformToEventsConfig assumed a list and called .map on it, so reading remote configuration (app config link, deploy diffing) would throw once an app's modules are stored in the new shape. Normalize the subscription value to a list before stripping the server-managed identifier. Multiple single-subscription modules then deep-merge back into one TOML subscription array through the existing remoteAppConfigurationExtensionContent union-array merge, exactly like webhook_subscription modules do today. Legacy aggregate modules are unaffected. Assisted-By: devx/8f786480-3b9f-4b46-b2d1-8c99989471b2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Core is migrating
eventsapp modules to a one-module-per-subscription structure (dual contract): each module's config carriesevents.subscriptionas a single object instead of a list, with the module uid derived from the subscription handle.transformToEventsConfigassumessubscriptionis always a list and calls.mapon it, so reading remote configuration (app config link, deploy config diffing) throws a TypeError once an app's stored modules use the new shape.WHAT is this pull request doing?
events.subscriptionvalue to a list (object → one-element list) before stripping the server-managedidentifier.[[events.subscription]]array through the existingremoteAppConfigurationExtensionContentunion-array merge — the same mechanismwebhook_subscriptionmodules use today.Known follow-up (out of scope): subscription ordering after merging N remote modules follows server module order, which may produce cosmetic diff noise in
app config linkoutput. Webhooks solves this with sort-on-parse (mergeAllWebhooks); events can adopt the same if it becomes a problem.How to test your changes?
pnpm vitest run packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts(12 tests, includes single-object wrap + N-module merge)shopify app config linkround-trips the TOML unchanged.Measuring impact
Checklist