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

Refactor to Next.js 13 and app directory #53

Merged
merged 6 commits into from Nov 13, 2022

Conversation

gfortaine
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Oct 18, 2022

@gfortaine is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@gfortaine gfortaine force-pushed the refactor/layouts branch 2 times, most recently from 7bf5536 to 6e350b2 Compare October 18, 2022 14:58
components/comment.js Outdated Show resolved Hide resolved
app/ssr/page.js Outdated Show resolved Hide resolved
app/ssr/page.js Outdated Show resolved Hide resolved
app/layout.js Outdated Show resolved Hide resolved
@leerob leerob changed the title refactor: Layouts RFC 🎉 Refactoring Oct 18, 2022
@vercel
Copy link

vercel bot commented Oct 18, 2022

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

Name Status Preview Updated
next-rsc-news ✅ Ready (Inspect) Visit Preview Nov 13, 2022 at 2:09AM (UTC)

@gfortaine gfortaine force-pushed the refactor/layouts branch 3 times, most recently from 11b3dde to 7b14cf3 Compare October 31, 2022 07:54
@leerob
Copy link
Member

leerob commented Nov 6, 2022

Hey! I don't have push access here. I made a PR: gfortaine#1

@gfortaine
Copy link
Contributor Author

@leerob PR merged 🎉

@gfortaine gfortaine force-pushed the refactor/layouts branch 11 times, most recently from 6209956 to 0016859 Compare November 12, 2022 18:07
@leerob
Copy link
Member

leerob commented Nov 12, 2022

Hey! Sorry one thing I should have been more clear about - I think we can ditch all of the CSR/slow examples and just do the RSC/streaming one as the main index page. I started down this route in my PR 👍

@gfortaine gfortaine force-pushed the refactor/layouts branch 6 times, most recently from 30669ad to 554bf2a Compare November 12, 2022 22:57
@gfortaine
Copy link
Contributor Author

@leerob Copy that 👍 MR updated accordingly :

  • bugfixes :
    • use react@^18.3.0-next (fix html mismatch errors between client & server + parentNode not found from loading.js)
    • remove fetchData in components/item.js
    • remove delay from fetchData function call in app/page.js
  • CSS modules everywhere possible
  • various code refactoring

@leerob leerob changed the title Refactoring Refactor to Next.js 13 and app directory Nov 12, 2022
@vercel-team
Copy link

vercel-team commented Nov 13, 2022

📝 Changed routes:

6 deleted routes:

  • /csr
  • /
  • /item.server
  • /rsc.server
  • /slow.server
  • /ssr

Commit cc3d9b3 (https://next-rsc-news-13vw4p4as.vercel.sh).

@gfortaine
Copy link
Contributor Author

@leerob Just discovered a very annoying bug by finishing this MR (to get the id searchParam in ItemPage Server Component) : vercel/next.js#42856

@leerob
Copy link
Member

leerob commented Nov 13, 2022

Thank you so much for your help here! Sending you an email for some swag 😄 This PR is already getting fairly large, so let's go ahead and merge forward and address bugs as we see them.

For example, I think there should be no surrounding padding on mobile. But it's minor, and good that this repo gets updated so folks don't try exploring the old version that won't work 😄

@mb21
Copy link

mb21 commented Nov 17, 2022

Hello! any particular reason you're using a plain old html <a> tag instead of the Next <Link component? This is essentially a MPA. From reading the beta docs for app directory, I understood the recommended approach to still be SPA? Thanks for clarification!

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.

None yet

4 participants