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.

Sign up for GitHub

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

Jump to bottom

Add support for Next.js >= 13.2 #6

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add support for Next.js >= 13.2 #6

wants to merge 6 commits into from
+6 −3

Conversation

yoieh
Copy link

@yoieh yoieh commented Apr 10, 2023 •
edited by sam3d
Loading

@yoieh yoieh mentioned this pull request Apr 10, 2023
Open
sam3d
sam3d requested changes Apr 10, 2023
View reviewed changes
Copy link
Owner

@sam3d sam3d left a comment

Choose a reason for hiding this comment

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

Excellent spot, thank you! Please could you also update the README.md with the tested version?

@yoieh
Copy link
Author

yoieh commented Apr 10, 2023

I have updated the Compatibility section of the readme.
This still doesn't give full support for the new app directory with the new Route Handlers.

@yoieh yoieh mentioned this pull request Apr 10, 2023
Open
@yoieh yoieh requested a review from sam3d April 10, 2023 13:17
@sam3d
Copy link
Owner

sam3d commented Apr 10, 2023 •
edited
Loading

The typescript type check fails because it needs to be aware of both possible versions of NextNodeServer. I'd like to ensure it doesn't

@yoieh
Copy link
Author

yoieh commented Apr 10, 2023 •
edited
Loading

I see,
There are more issues here.

ensurePage only exists on the DevServer in version 13.3.0 and not at all NextNodeServer.
This PR will never work in production.

But looks like ensureEdgeFunction exists in both 13.1.6 and 13.3.0 and works in both dev and prod.

@yoieh
Copy link
Author

yoieh commented Apr 10, 2023

I still kept the ensureApiPage for now..
but it look like you may be able to just use the ensureEdgeFunction from here on.

Not sure when i was added so can not garanti it works with a version lower than 13.1.6.
But this will "make typescript not fail".

sam3d
sam3d requested changes Apr 10, 2023
View reviewed changes
Copy link
Owner

@sam3d sam3d left a comment

Choose a reason for hiding this comment

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

Oooh I like this solution! Please could you change it to just ensureEdgeFunction and I will give it a test and merge?

@yoieh
Changed out ensurePage and to just use ensureEdgeFunction
a689e2f 
Changed ensurePage to use ensureEdgeFunction

Change it to just ensureEdgeFunction
@yoieh yoieh requested a review from sam3d April 10, 2023 16:13
@sam3d sam3d changed the title Addes support for NetxtJS > 13.1.6 Add support for Next.js >= 13.2 Apr 10, 2023
@tarik02 tarik02 mentioned this pull request Jul 17, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sam3d sam3d Awaiting requested review from sam3d

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

run next dev found some error
2 participants
@yoieh @sam3d