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

[examples] Add Next.js 13 starter project #34983

Closed
wants to merge 12 commits into from

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Nov 2, 2022

Related to #34905. The goal is to add a new example using Next.JS's app directory (with client components only).

TODOs & open questions I have:

@mnajdova mnajdova added examples Relating to /examples proof of concept Studying and/or experimenting with a to be validated approach labels Nov 2, 2022
@mui-bot
Copy link

mui-bot commented Nov 2, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-34983--material-ui.netlify.app/

No bundle size changes

Generated by 🚫 dangerJS against e31d2c8

@mnajdova mnajdova changed the title [examples] Add Next.js 13 started project [examples] Add Next.js 13 starter project Nov 2, 2022
@@ -0,0 +1,29 @@
'use client';
Copy link
Member Author

Choose a reason for hiding this comment

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

We shouldn't move forward with this PR before we are able to get rid of this directive. The layout shouldn't be a client component.

Copy link
Member

@oliviertassinari oliviertassinari Dec 1, 2022

Choose a reason for hiding this comment

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

A couple of thoughts:

  1. Server-side component is not the only benefit of using Next.js's beta app folder. They list 4 in https://nextjs.org/blog/next-13#new-app-directory-beta:

Screenshot 2022-12-01 at 23 48 22

It could maybe make sense to provide a WIP demo. For example, to benefit from nested routes. Mantine as a WIP example mantinedev/mantine#2815 (comment). It could also resonate with https://mui.zendesk.com/agent/tickets/5883

we are hoping to use Next 13 with the app directory and layout.tsx files, to keep from having to retrofit the app after the fact

  1. We used the font logic to remove layout shifts in the examples [examples] Next.js examples v13 - fonts #34971. I guess we should do the same with our own docs?
  2. Server-side components can't use React.useContext, React.useState, React.useRef. So, let's say the issue is solve with emotion, we would also have to decide when the component needs to be client-side. There are cases, e.g. a button that could work server-side only even if it has a ton of hooks inside. So maybe, we will need to have a custom React.useState, React.useRef utils that do a no-op for these cases. https://twitter.com/olivtassinari/status/1601884631426633728
  3. Next.js not supporting the context for the server-side components doesn't make sense to me. Can I use Server Component with a global Context? vercel/next.js#42301 cover a bit of the problem. It feels like it was created in a bubble, without considering how real-life app constraints are. For a given request, a theme is static, a locale is static, etc.

Copy link

@madaher-dev madaher-dev Dec 2, 2022

Choose a reason for hiding this comment

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

It feels like it was created in a bubble, without considering how real-life app constraints are. For a given request, a theme is static, a locale is static, etc.

Exactly!

Copy link
Member

@oliviertassinari oliviertassinari Jan 30, 2023

Choose a reason for hiding this comment

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

We shouldn't move forward with this PR before we are able to get rid of this directive. The layout shouldn't be a client component.

Per #34905 (comment), I think that it would be great to merge to have this demo, even if with use client. The /app folder has more to offer beyond server-side components.

@mnajdova
Copy link
Member Author

It's been already added in a different PR.

@mnajdova mnajdova closed this Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Relating to /examples proof of concept Studying and/or experimenting with a to be validated approach
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants