-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
major: full dynamic pages and plain next.js #5426
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@ovflowd is attempting to deploy a commit to the OpenJS Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Hey! That's great. |
No, it is not smarter to have a A specific content folder brings no benefits and only adds complexity. A content folder also doesn't solve the original issue, so not even sure why you're mentioning it here.
I know how this works, we did it on Gatsby a long time ago. But Next.js works differently, and even tho we could load the content dynamically only from a content folder, it is kinda anti-intuitive. The current approach exists to support non-localized pages to have their source content to be shown and still be localised. This works because:
Also, gentle reminder that you already asked me this a few times, and I find it kinda annoying that you keep insisting on it (if that was not your intention, then I'm sorry 😅) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
Thanks Claudio for your answer.
Updates on the PR
|
cc @nodejs/website having reviews would be nice here :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An initial manual pass - hoping to run the code this weekend on my Windows machine
Going to rebase and update based on code-review soon. |
Hey, @nodejs/website I've updated this PR and now it contains the full solution of:
Everything is working as expected now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to run the code locally again but thought I'd submit this initial review
Co-authored-by: Brian Muenzenmeyer <[email protected]> Signed-off-by: Claudio Wunder <[email protected]>
@aymen94 I think it was premature to get such a massive PR merged without more reviews from other folks. But I guess it's fine 😅 |
@ovflowd I understand your point regarding the number of reviews for merging a completely new development. While four reviews may seem like enough, I appreciate your suggestion to pay attention. 😁 |
Description
This PR implements the Discussion started here on leaving Nextra in favour of a vanilla Next.js experience.
Truth be told, as mentioned in the discussion, we finally matured so that we don't need Nextra anymore.
This PR implements an engine that allows Incremental Builds (SSR) and full Static Builds of non-localised pages with localised context(s).
Leveraging Next.js Dynamic Routes feature (Static Paths + On-demand Static Page Generation).
This also uses MDX Remote for the compilation of MDXm for Next.js on build-time and on-demand.
How it works
MDXRemote
NextraTheme
with necessary propsRelated Issues
Check List
npx turbo lint
to ensure the code follows the style guide. And runnpx turbo lint:fix
to fix the style errors if necessary.npx turbo format
to ensure the code follows the style guide.npx turbo test
to check if all tests are passing, and/ornpx turbo test:snapshot
to update snapshots if I created and/or updated React Components.