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

Add order list for logged user #1011

Merged
merged 9 commits into from
Nov 11, 2023
Merged

Add order list for logged user #1011

merged 9 commits into from
Nov 11, 2023

Conversation

zaiste
Copy link
Contributor

@zaiste zaiste commented Nov 10, 2023

No description provided.

Copy link

vercel bot commented Nov 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
storefront ✅ Ready (Inspect) Visit Preview Nov 11, 2023 4:05pm

@typeofweb
Copy link
Contributor

Could you rebase @zaiste ?

src/app/(main)/orders/page.tsx Outdated Show resolved Hide resolved
src/app/(main)/orders/page.tsx Outdated Show resolved Hide resolved
src/app/(main)/orders/page.tsx Outdated Show resolved Hide resolved
src/app/(main)/orders/page.tsx Outdated Show resolved Hide resolved
src/app/(main)/orders/page.tsx Outdated Show resolved Hide resolved
src/app/(main)/orders/page.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@typeofweb typeofweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace slate with neutral

@typeofweb
Copy link
Contributor

This doesn't look too good:

Screenshot 2023-11-10 at 17 09 26

package.json Outdated Show resolved Hide resolved
Comment on lines 3 to 4
export default function OrderPage() {
const OrderList = dynamic(() => import("../../../ui/components/OrderList"), { ssr: false });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be outside the component body?

Suggested change
export default function OrderPage() {
const OrderList = dynamic(() => import("../../../ui/components/OrderList"), { ssr: false });
const OrderList = dynamic(() => import("../../../ui/components/OrderList").then((m) => m.OrderList), { ssr: false });
export default function OrderPage() {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmiszy named export doesn't work unfortunately

Copy link
Contributor

@typeofweb typeofweb Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does work, we're already using it in other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmiszy can you check on your side?

CleanShot 2023-11-11 at 14 00 10@2x

(with re-created .next/)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed there's a bug in Next.js :D Adding "use client" to the page.tsx fixes it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D

Yeah, but I'd prefer not to set the entire page as client. Let's keep it as default for now with an eslint exception, there's an open bug report in the Next.js repo, let's track it and fix it once it's done upstream. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole page is client anyway because you're only rendering a single dynamic component.

src/ui/components/OrderList.tsx Outdated Show resolved Hide resolved
src/ui/components/OrderList.tsx Outdated Show resolved Hide resolved
src/ui/components/OrderList.tsx Outdated Show resolved Hide resolved
@typeofweb typeofweb merged commit 6180d07 into canary Nov 11, 2023
3 checks passed
@typeofweb typeofweb deleted the order-list branch November 11, 2023 16:09
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 this pull request may close these issues.

2 participants