[Feature] Remove chrome extension dependency for post-purchase #5584
Conversation
carolinesalib
force-pushed
the
post-purchase/remove-chrome-extension-dependency
branch
2 times, most recently
from
033a281 to
d77dccf
Compare
carolinesalib
force-pushed
the
post-purchase/remove-chrome-extension-dependency
branch
from
d77dccf to
0073d19
Compare
carolinesalib
force-pushed
the
post-purchase/remove-chrome-extension-dependency
branch
from
0073d19 to
1588bd0
Compare
carolinesalib
force-pushed
the
post-purchase/remove-chrome-extension-dependency
branch
from
1588bd0 to
9e0ac78
Compare
Coverage report
Test suite run success2192 tests passing in 958 suites. Report generated by 🧪jest coverage report action from 69fee77 |
| @@ -88,6 +88,11 @@ export async function getUIExtensionPayload( | |||
| async function getExtensionPoints(extension: ExtensionInstance, url: string) { | |||
| const extensionPoints = extension.configuration.extension_points as DevNewExtensionPointSchema[] | |||
|
|
|||
| if (extension.type === 'checkout_post_purchase') { | |||
| // Mock target for post-purchase in order to get the right extension point redirect url | |||
| return [{target: 'purchase.post.render'}] as DevNewExtensionPointSchema[] | |||
There was a problem hiding this comment.
There is no purchase.post.render target. Post-purchase extension has two extension points however no targets as there is no option to select where the extension will render (aka it is static). However, following an idea introduced by @vividviolet on this PR I decided to re-use the implicit post-purchase mock target to try to reuse most of the logic that was already used by checkout ui extensions. Let me know if you have any thoughts about this approach
There was a problem hiding this comment.
@carolinesalib isn't there a new target for post purchase when it's a ui_extension right? Is that what we're using here?
There was a problem hiding this comment.
Another idea - couldn't we transform the extension's schema here so that it correctly has the right shape and then we don't need this special conditional?
There was a problem hiding this comment.
@vividviolet I don't think there is a target for post-purchase extensions. It doesn't use the same framework as ui_extension 🤔
There was a problem hiding this comment.
Ah ok, I thought these already have corresponding new targets in the shared ui_extension specification. Will we never migrate post_purchase then?
There was a problem hiding this comment.
It's been in our roadmap for years, but it always ends up getting pushed to the next year 🥲
It seems like we'll finally start working on a post-purchase migration later this year/beginning of next year, but no hard plans just yet - we are discussing when we could start working on it.
carolinesalib
changed the title
|
We detected some changes at packages/*/src and there are no updates in the .changeset. |
carolinesalib
requested review from
kumar303,
robin-drexler,
denislins,
lemonmade,
isaacroldan and
vividviolet
| // This can never be null because we always generate it | ||
| // whenever there is an extension point targeting Post Purchase | ||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
| rawUrl.pathname = options.checkoutCartUrl! |
There was a problem hiding this comment.
We should try this case:
- Starting a dev-session (with the new API) for an app without extensions
- Adding a new post-purchase extension while dev is still running
There was a problem hiding this comment.
what is the expectation here? that the dev dashboard reloads with 2 extensions for testing? 🤔
| /** | ||
| * The URL of the websocket used for hot reload | ||
| */ | ||
| websocketURL?: string |
There was a problem hiding this comment.
Do you need this here? the caller of devUIExtensions is never including the websocketURL as part of ExtensionDevOptions
There was a problem hiding this comment.
And ExtensionsPayloadStoreOptions is already a ExtensionDevOptions + websocketURL
There was a problem hiding this comment.
I added that because I was getting this error when trying to get websocketURL.
Is there a better way to work around this? Typescript can still be challenging for me so if you have an example I would appreciate it 🙏
There was a problem hiding this comment.
it's beacuse we are using ExtensionDevOptions in many places where we should be using ExtensionsPayloadStoreOptions.
I've pushed a commit fixing this :)
carolinesalib
force-pushed
the
post-purchase/remove-chrome-extension-dependency
branch
from
c50683c to
5face64
Compare
WHY are these changes introduced?
Fixes https://github.com/Shopify/temp-project-mover-syfiawoo-20250403105126/issues/68
The checkout post-purchase extension depends on the Shopify Post-purchase Developer Tools Chrome Extension that is currently unavailable. What the chrome extension does is to inject parameters to the checkout allowing developers to test their post-purchase extension on their development store. With this PR we are removing the dependency of the chrome extension and passing the required parameters via cart permalink when running the dev server.
WHAT is this pull request doing?
As a bonus, this PR also makes the
checkout_post_purchaseextension more consistent with the checkoutcheckout_ui_extensionpatterns.How to test your changes?
shopify app initand thenshopify app generate extension --template post_purchase_ui --name my-post-purchase-ui-extensionpnpm shopify app dev --path PATH_TO_EXTENSIONon cli repoPost-release steps
Yes, I do need to update some docs before merging this PR but I will wait on feedback first since this is my first time contributing to this repo.
Measuring impact
How do we know this change was effective? Please choose one:
Checklist