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

Update for Fastify v5 #957

Open
wants to merge 2 commits into
base: next
Choose a base branch
from
Open

Update for Fastify v5 #957

wants to merge 2 commits into from

Conversation

synapse
Copy link

@synapse synapse commented Mar 20, 2024

Ref: fastify/fastify#5116

⚠️ There is one small issue with this one, and it's the NextJS dependency. Upgrading it to v14.x will ignore the basePath option. Even thought the basePath is still present in their config docs and typings it does not work. I am unsure if this doc saying that it was removed might have something to do with this issue.
I am open to discuss what would be the best way to fix this one.

Checklist

CC @simoneb

@simoneb simoneb changed the base branch from master to next March 20, 2024 16:40
@simoneb
Copy link
Contributor

simoneb commented Mar 20, 2024

created and rebased

@simoneb
Copy link
Contributor

simoneb commented Mar 20, 2024

tests are broken

@synapse
Copy link
Author

synapse commented Mar 21, 2024

tests are broken

I think we need to drop node 14 and 16 for this one

@simoneb
Copy link
Contributor

simoneb commented Mar 21, 2024

tests are broken

I think we need to drop node 14 and 16 for this one

it was always part of the original plan, wasn't it? fastify/fastify#5116

Most likely this will simply mean updating dependencies, dropping Node <= 16, and updating CI configuration accordingly

@gurgunday gurgunday changed the base branch from next to master April 14, 2024 19:58
@gurgunday gurgunday changed the base branch from master to next April 14, 2024 19:58
@@ -1,6 +1,4 @@
100: true
Copy link
Member

Choose a reason for hiding this comment

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

Just one question:

Why did we go to disable-coverage from 100?

Maybe it works without disable-coverage?

@gurgunday
Copy link
Member

Hey, can you pull form main? I can't realistically resolve the package-lock conflicts on the web interface...

And if you could also add node 22 to the test matrix it would be great

Then this is good to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

None yet

3 participants