vendure-ecommerce / storefront-remix-starter

A storefront starter kit for Vendure built with Remix
https://remix-storefront.vendure.io
176 stars 93 forks source link

Upgrading to Vendure 2.0 #40

Closed michaelbromley closed 1 year ago

michaelbromley commented 1 year ago

Vendure v2.0 is in beta and will be released fairly soon.

This issue is intended to collect notes on what needs to change in this project to support Vendure v2.

One I found so far:

this line needs to change to:

gql`
-  mutation setOrderShippingMethod($shippingMethodId: ID!) {
+  mutation setOrderShippingMethod($shippingMethodId: [ID!]!) {
    setOrderShippingMethod(shippingMethodId: $shippingMethodId) {
      ...OrderDetail
      ... on ErrorResult {
        errorCode
        message
      }
    }
  }
`;
kyunal commented 1 year ago

How should we go about versioning? For now we could set up a V2 branch that, once V2 is officially out, we can merge into main.

I'm not sure whether or not we should preserve any V1 compatibility. I'm not even sure if it is important. I think it would be fine just linking the last commit & project tree before V2 support was merged, but that would mean no patches could be made by anybody concerned.

DanielBiegler commented 1 year ago

I personally think we shouldnt keep backwards compatability. In my mind this repo should be state of the art for Vendure so new people coming in always have a clean reference and demo with the latest features. I like linking to the last compatible v1 commit but not providing patches.

I'd just open a v2 branch after v2 officially drops and do everything in one swoop after the needed changes are finalized in this issue.

michaelbromley commented 1 year ago

Yeah I'd agree with just supporting the latest Vendure release on head. Since this is a "starter" rather than a library, the usage pattern would be to clone it at some point in time, and then just modify to your needs, after which point it belongs to you and you are in charge of maintaining & updating. Link to last v1 is a good idea though.

zolzaya commented 1 year ago

I'm using vendure 2.0 beta but I found a bug. When I go to the orders page, I get the following error.

[1] Error: {"message":"Cannot query field \"fulfillments\" on type \"OrderLine\". Did you mean \"fulfillmentLines\"?","locations":[{"line":29,"column":11}]}
[1]     at /Users/zoloo/code/superb/storefront/app/graphqlWrapper.ts:109:13
[1]     at processTicksAndRejections (node:internal/process/task_queues:95:5)
[1]     at loader12 (/Users/zoloo/code/superb/storefront/app/route-containers/account/orders.server.ts:27:8)
[1]     at Object.callRouteLoaderRR (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/server-runtime/dist/data.js:41:16)
[1]     at callLoaderOrAction (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:3364:16)
[1]     at async Promise.all (index 0)
[1]     at loadRouteData (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:2878:19)
[1]     at queryImpl (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:2657:20)
[1]     at Object.queryRoute (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:2597:18)
[1]     at handleDataRequestRR (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/server-runtime/dist/server.js:81:20)
[1] GET /account/orders?_data=routes%2Faccount%2Forders 500 - - 32.383 ms

how to fix?

DanielBiegler commented 1 year ago

@zolzaya This Storefront Template does not support v2 as of yet. Some internal data structures change between v1 and v2. For example in v1 you might get an Object and in v2 you might get an Array. If you want to use v2 you will have to fix all those differences yourself. Stuff like iterating or accessing non existing fields. When v2 drops I will look into that, probably, but for now youre on your own.

dylviz commented 1 year ago

I'm using vendure 2.0 beta but I found a bug. When I go to the orders page, I get the following error.

[1] Error: {"message":"Cannot query field \"fulfillments\" on type \"OrderLine\". Did you mean \"fulfillmentLines\"?","locations":[{"line":29,"column":11}]}
[1]     at /Users/zoloo/code/superb/storefront/app/graphqlWrapper.ts:109:13
[1]     at processTicksAndRejections (node:internal/process/task_queues:95:5)
[1]     at loader12 (/Users/zoloo/code/superb/storefront/app/route-containers/account/orders.server.ts:27:8)
[1]     at Object.callRouteLoaderRR (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/server-runtime/dist/data.js:41:16)
[1]     at callLoaderOrAction (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:3364:16)
[1]     at async Promise.all (index 0)
[1]     at loadRouteData (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:2878:19)
[1]     at queryImpl (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:2657:20)
[1]     at Object.queryRoute (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/router/router.ts:2597:18)
[1]     at handleDataRequestRR (/Users/zoloo/code/superb/storefront/node_modules/@remix-run/server-runtime/dist/server.js:81:20)
[1] GET /account/orders?_data=routes%2Faccount%2Forders 500 - - 32.383 ms

how to fix?

Any luck on this? I'm also seeing this

zolzaya commented 1 year ago

Hi, I fixed it. Here is the how:

On the app/generated/graphql.ts file remove code between 3892-3895. Here is the removed code:

          fulfillments {
            updatedAt
            state
          }

Above code is not used in the frontend. So I removed it.

dylviz commented 1 year ago

Hi, I fixed it. Here is the how:

On the app/generated/graphql.ts file remove code between 3892-3895. Here is the removed code:

          fulfillments {
            updatedAt
            state
          }

Above code is not used in the frontend. So I removed it.

I thought this was it, but actually fullfillments is used in OrderHistoryItem.tsx, but may not be critical at this point for me as it looks like it's just used to report the state of the fulfillment

DanielBiegler commented 1 year ago

@zolzaya @dylviz Removing code from the auto generated file doesnt really fix the root problem.

After generating types again, i.e. after you make an update or run codegen, it will break again because the types will be regenerated.

The types get generated because of here: https://github.com/vendure-ecommerce/storefront-remix-starter/blob/75eb880052d7f76b2026fc917cf545996012e3ac/app/providers/customer/customer.ts#L99-L102

Secondly, they are used in the OrderHistoryItem here https://github.com/vendure-ecommerce/storefront-remix-starter/blob/75eb880052d7f76b2026fc917cf545996012e3ac/app/components/account/OrderHistoryItem.tsx#L106-L114

The quick fix for if you dont need the fulfillment info is to

  1. remove it from the customer query activeCustomerOrderList
  2. rerun codegen
  3. remove it from OrderHistoryItem

But remember that youre then branching off of this repo and will get conflicts when theres updates.

dylviz commented 1 year ago

@zolzaya @dylviz Removing code from the auto generated file doesnt really fix the root problem.

After generating types again, i.e. after you make an update or run codegen, it will break again because the types will be regenerated.

The types get generated because of here:

https://github.com/vendure-ecommerce/storefront-remix-starter/blob/75eb880052d7f76b2026fc917cf545996012e3ac/app/providers/customer/customer.ts#L99-L102

Secondly, they are used in the OrderHistoryItem here

https://github.com/vendure-ecommerce/storefront-remix-starter/blob/75eb880052d7f76b2026fc917cf545996012e3ac/app/components/account/OrderHistoryItem.tsx#L106-L114

The quick fix for if you dont need the fulfillment info is to

1. remove it from the customer `query activeCustomerOrderList`

2. rerun codegen

3. remove it from `OrderHistoryItem`

But remember that youre then branching off of this repo and will get conflicts when theres updates.

It seems like a mismatch between the new backend type and the old frontend. I can see the backend Orderline type (in order.type.graphql) doesn't have fulfillments anymore, but rather fulfillmentLines: [FulfillmentLine!] under type OrderLine

So it seems like the correct way to fix this would be to remove fulfillment{} as you suggested then utilize fulfillmentLines to try to pull the same info. Does that sound like the right path?

zolzaya commented 1 year ago

@DanielBiegler you're right it's the quick fix.

DanielBiegler commented 1 year ago

So it seems like the correct way to fix this would be to remove fulfillment{} as you suggested then utilize fulfillmentLines to try to pull the same info. Does that sound like the right path?

@dylviz Yes. But remember that there are more differences between v1 and v2. This is not the only change. You will run into further roadblocks later.

dylviz commented 1 year ago

So it seems like the correct way to fix this would be to remove fulfillment{} as you suggested then utilize fulfillmentLines to try to pull the same info. Does that sound like the right path?

@dylviz Yes. But remember that there are more differences between v1 and v2. This is not the only change. You will run into further roadblocks later.

We will get to know each other very well through all the debugging :) lol or are you trying to recommend that I wait for an update? I'm OK with either!

kyunal commented 1 year ago

We will get to know each other very well through all the debugging :) lol or are you trying to recommend that I wait for an update? I'm OK with either!

I intend to get the starter ready before the Vendure V2 release. Also, V2 API changes are documented and discussed here: https://github.com/vendure-ecommerce/vendure/discussions/1991


cc @michaelbromley is there a demo instance for V2 yet? Or do you intend to upgrade the existing instance once V2 is out?

michaelbromley commented 1 year ago

Hi @kyunal, there's no live demo instance of v2 currently. This will probably be the case until the final release in a few weeks time. But getting a local instance up and running is just a matter of npx @vendure/create@next.

DanielBiegler commented 1 year ago

@michaelbromley hey there! Are the data structures for v2 finalized now? Or is there still a pending thing? Sry if that has been answered before, I havent seen it. If it's finalized I'd be down to port this storefront to v2 @kyunal

Edit: Just seen the new Video where Michael literally says "all the major breaking changes are done". :rofl:

Yayy congratz! :rocket:

michaelbromley commented 1 year ago

Emphasis on the word "major" :D

I cannot guarantee that there are no more breaks, but I don't predict any major ones that would then require a significant amount of work.

kyunal commented 1 year ago

@michaelbromley hey there! Are the data structures for v2 finalized now? Or is there still a pending thing? Sry if that has been answered before, I havent seen it. If it's finalized I'd be down to port this storefront to v2 @kyunal

Edit: Just seen the new Video where Michael literally says "all the major breaking changes are done". rofl

Yayy congratz! rocket

I'd be very grateful for your contribution! Feel free to make another (draft) PR as soon as you start working on it, then we can all test it and provide feedback.

dylviz commented 1 year ago

@michaelbromley hey there! Are the data structures for v2 finalized now? Or is there still a pending thing? Sry if that has been answered before, I havent seen it. If it's finalized I'd be down to port this storefront to v2 @kyunal Edit: Just seen the new Video where Michael literally says "all the major breaking changes are done". rofl Yayy congratz! rocket

I'd be very grateful for your contribution! Feel free to make another (draft) PR as soon as you start working on it, then we can all test it and provide feedback.

Let me know if I can contribute to this PR as well. I set some time aside this week to get the front end working with V2 but don't want to create redundant work if it's already being started

DanielBiegler commented 1 year ago

@zolzaya @dylviz I fixed the order history in #44 - Have you encountered problems elsewhere?

Setting the shipping method still works. The type changed to be able to accept an array but string still works too.

dylviz commented 1 year ago

@zolzaya @dylviz I fixed the order history in #44 - Have you encountered problems elsewhere?

Setting the shipping method still works. The type changed to be able to accept an array but string still works too.

Hi! OK, I will try out. I believe there are quite a few things that need adjustment. If I run yarn generate with the new backend, I see about 76 incompatibilities. I can comment feedback on the PR discussion if that's OK. I also set some time aside this week to work on this so if there's anything I can do to contribute to this PR, just let me know. I'll have time.

kyunal commented 1 year ago

Vendure V2 support by @DanielBiegler is merged into https://github.com/vendure-ecommerce/storefront-remix-starter/tree/vendure-v2 until a V2 demo instance is available

kyunal commented 1 year ago

Closing this as V2 support is finally merged into the master branch :rocket: If anybody's wondering, https://github.com/vendure-ecommerce/storefront-remix-starter/tree/75eb880052d7f76b2026fc917cf545996012e3ac is the last supported commit for V1.