Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading to Vendure 2.0 #40

Closed
michaelbromley opened this issue Apr 21, 2023 · 23 comments · Fixed by #44
Closed

Upgrading to Vendure 2.0 #40

michaelbromley opened this issue Apr 21, 2023 · 23 comments · Fixed by #44

Comments

@michaelbromley
Copy link
Member

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
Copy link
Collaborator

kyunal commented Apr 24, 2023

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link

zolzaya commented Apr 26, 2023

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
Copy link
Contributor

@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
Copy link

dylviz commented May 19, 2023

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
Copy link

zolzaya commented May 19, 2023

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
Copy link

dylviz commented May 19, 2023

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
Copy link
Contributor

@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:

fulfillments {
updatedAt
state
}

Secondly, they are used in the OrderHistoryItem here

{/* Shipment status */}
<span className="text-gray-500 text-xs mt-2 tracking-wide">
{line.fulfillments && line.fulfillments.length === 0 && ('Not shipped yet')}
{line.fulfillments?.map((f, i) => (
<span key={i} className="block" title={(new Date(f.updatedAt)).toLocaleString()}>
{f.state}: {new Intl.DateTimeFormat(undefined, { dateStyle: "medium" }).format(new Date(f.updatedAt))}
</span>
))}
</span>

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
Copy link

dylviz commented May 19, 2023

@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:

fulfillments {
updatedAt
state
}

Secondly, they are used in the OrderHistoryItem here

{/* Shipment status */}
<span className="text-gray-500 text-xs mt-2 tracking-wide">
{line.fulfillments && line.fulfillments.length === 0 && ('Not shipped yet')}
{line.fulfillments?.map((f, i) => (
<span key={i} className="block" title={(new Date(f.updatedAt)).toLocaleString()}>
{f.state}: {new Intl.DateTimeFormat(undefined, { dateStyle: "medium" }).format(new Date(f.updatedAt))}
</span>
))}
</span>

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
Copy link

zolzaya commented May 19, 2023

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

@DanielBiegler
Copy link
Contributor

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
Copy link

dylviz commented May 19, 2023

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
Copy link
Collaborator

kyunal commented May 19, 2023

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: vendure-ecommerce/vendure#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
Copy link
Member Author

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
Copy link
Contributor

DanielBiegler commented May 19, 2023

@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". 🤣

Yayy congratz! 🚀

@michaelbromley
Copy link
Member Author

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
Copy link
Collaborator

kyunal commented May 19, 2023

@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
Copy link

dylviz commented May 20, 2023

@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
Copy link
Contributor

@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
Copy link

dylviz commented May 22, 2023

@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
Copy link
Collaborator

kyunal commented Jun 1, 2023

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
Copy link
Collaborator

kyunal commented Jun 7, 2023

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

@kyunal kyunal closed this as completed Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants