Shopify / ui-extensions

MIT License
260 stars 36 forks source link

We can't share code between TYP and OSP #1885

Open lucasprag opened 5 months ago

lucasprag commented 5 months ago

Please list the package(s) involved in the issue, and include the version you are using

"@shopify/app": "3.57.1",
"@shopify/checkout-ui-extensions-react": "0.27.3", // for retro-compatibility purposes. We're upgrading to the new package, but facing the issue below
"@shopify/cli": "3.57.1",
"@shopify/ui-extensions": "2024.4.1",
"@shopify/ui-extensions-react": "2024.4.1",

Describe the bug

We can't share code between Thank You Page and Order Status Page extensions after upgrading to the new API/package.

We're upgrading our extensions from using the deprecated @shopify/checkout-ui-extensions-react to the new @shopify/ui-extensions-react package while also moving up from the 2023-10 API version to 2024-01 or 2024-04

At Smile, we have several Shopify extensions in production and we have plans to add more or/and add features to the existing ones. It's only natural that these extensions that share features to share code as well (such as logic or hooks to get the authenticated customer/shopper, get metafields, etc).

However, we're facing a problem:

Thank You Page (purchase.thank-you.block.render)

Order Status Page -- (customer-account.order-status.block.render)

This means that the common ground code we share between between these two extensions can't work anymore because each extension requires it to be imported from a different surface/package.

Please let me know if I'm missing something here.

Steps to reproduce the behavior:

N/a because it's more of a conceptual limitation from the Shopify packages.

Expected behavior

We should be able to import any hook that exists in both extensions from a common surface or package and have it work regardless of the target we're rendering the extension. This was the case up until now.

Screenshots

Thank you Page

typ using checkout typ using customer-account

Order Status Page

osp using checkout osp using customer-account

Additional context

jamesvidler commented 5 months ago

Hi @lucasprag,

Yes, you can't use the same hooks or components anymore because they fundamentally come from a different surface. This is good feedback to hear though and we appreciate your detailed explanation of the problem.

What we would recommend for now is to refactor your code so that you have two distinct entry points for each target, and then have some shared functions that take props that aren't aware of the surface and contain the main parts of your logic. The hooks and components will need to remain in these top-level extension components, but many of the other parts can be re-used.

I realize this is not ideal, and we'll be considering how we can improve up on this in the future.

andrewpye commented 4 months ago

shared functions that take props that aren't aware of the surface and contain the main parts of your logic

@jamesvidler this becomes horribly complex when TypeScript is being used, because each surface has its own type definitions that can't be used interchangeably. We have a number of existing UI extensions which are mostly on the checkout surface, but one of them is usable on both the thank-you and order status pages – which are now two separate surfaces (checkout and customer account). Each of those extensions uses the same set of shared hooks for purposes such as identifying the customer in our own system, amongst other things. We've had multiple developers spend days trying to work around this and still haven't arrived at a satisfactory solution: it's now seeming like as a result of this splitting of surfaces, our only feasible path forward is to either rebuild our existing extension almost from the ground up, or duplicating a large percentage of the code that was previously usable. Obviously neither of those paths are good, so with that in mind are you able to share any insight here into 1) how Shopify envisaged developers migrating existing extensions to account for this situation, and 2) if there is anything in the pipeline to smooth over this issue? I'd hate to invest the time needed to refactor our multiple existing extensions to satisfy this new separation of surfaces only for there to be a way of unifying them again announced shortly afterwards 😅 Thanks!

edhgoose commented 2 months ago

Hi @jamesvidler - can I confirm my understanding of the issue?

We have two extensions - one for the Checkout, one for Thank You/Order Status.

Within those two extensions we do some common pieces. I've attempted to pull that common code into a shared folder which exists in the root directory. That common code includes hooks like useShop or useCurrency.

I've found that since this morning (for reasons I cannot determine), that shared folder no longer works. I'm finding I get an error: You can only call this hook when running as a checkout UI extension.

On reviewing this issue, my understanding is that shared folder should not have worked anyway - and that in order to share the code, I need to ensure all the hooks and component specific code is kept within the extension.

Any shared code needs to be abstracted out to be hook agnostic - e.g. we pass in the result of the hooks as parameters.

Is that correct?

i.e. instead of:

const myFunction = () => {
    const shop = useShop();

    ... do something with shop
}

I need to do something like:

const myFunction = (shop) => {
    ... do something with shop
}

Is that right?

jamesvidler commented 2 months ago

Any shared code needs to be abstracted out to be hook agnostic - e.g. we pass in the result of the hooks as parameters.

That is correct. That would be the best way to do this currently.

edhgoose commented 2 months ago

Thanks @jamesvidler - I second what the others have raised in this thread, this is a significant limitation and fixing this would be extremely important to us.

Do you have any idea why the issue has occurred for me today? I don't recall doing anything different today. And it was working previously.