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

Patch node modules next headers not found #10302 #10304

Closed
wants to merge 5 commits into from

Conversation

OpenTextASOWSM
Copy link

☕️ Reasoning

This is the fix described in -
#10302

seems a reasonable number of people are hitting this issue.

#9385

🧢 Checklist

  • Documentation - no change to documentation, patch of some import statements
  • Tests - use existing tests
  • Ready to be merged

🎫 Affected issues

Cannot find module node_modules\next\headers - 10302

📌 Resources

#9385

Copy link

vercel bot commented Mar 13, 2024

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

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 15, 2024 4:54pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Mar 15, 2024 4:54pm

Copy link

vercel bot commented Mar 13, 2024

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

A member of the Team first needs to authorize it.

BjornHMS

This comment was marked as resolved.

@OpenTextASOWSM
Copy link
Author

Hi, anyone can review/approve?

Seems to be effecting others.

#9385 (comment)

@HazimAlper
Copy link

Can someone review this pr? It solved my problem in locally.

@willemmulder
Copy link

I was just hitting this issue. From what I see, everyone that uses /pages in Next.js that use auth() function within getServerSideProps() is affected. Thanks for the PR! :-) Can this be merged?

@OpenTextASOWSM
Copy link
Author

@ThangHuuVu can you review?

as a work around until merged might work.... worked for others

' this was fixed by transpiling next-auth in next.config.js

{
transpilePackages: ['next-auth'],
}

'

@ndom91
Copy link
Member

ndom91 commented Apr 25, 2024

While this may work, I have a feeling this is not the best way to go about this, changing the next imports to append the extension. There's gotta be a tsconfig option or something that fixes this, no? @balazsorban44

@ndom91
Copy link
Member

ndom91 commented Apr 26, 2024

Hey folks, so I talked to the team and theres another PR open that should take care of this as well actually. Looks like its only (hopefully temporarily) necessary for next imports, but it definitely isn't right. Might be a next.js issue 🤔

Anywayy, I'd appreciate if you guys could test if this PR fixes your issues as well and then we can merge that one 🙏

#9953

@OpenTextASOWSM
Copy link
Author

@ndom91 It looks like all three changes from this PR are a subset of where they found this issue in next-auth. It looks like they are doing the same thing... there were already cases like in packages/next-auth/src/lib/index.ts where the next-auth code was specifying .js for inclusions for env.js and actions.js both the PRs try to address it by following that pattern.

I'm not in a postion to test again for some time.

@OpenTextASOWSM
Copy link
Author

looks like the merge of the other code is blocked by security issue, if that isn't resolvable quickly since it's been 2 months maybe you can merge this.

@ndom91
Copy link
Member

ndom91 commented Apr 26, 2024

@OpenTextASOWSM thanks for the quick response.

Yeah so we were using this for internal imports already, for sure. But using it for external, i.e. 3rd party, packages is what feels wrong to me / the team.

Unfortunately it does seem to be necessary atm for next-related imports as well.

I cleaned up that other PR a bit so there are no more merge conflicts and theoretically it should be good to go 🙏

Woudl still appreciate some feedback from anyone else if they ran into this issue and coudl test that other PR to see if it fixes it for them without introducing any other issues

@balazsorban44
Copy link
Member

This is not the correct solution. third-party imports should not need an import specifier. The issue here is that Next.js currently does not have an exports field. There is some ongoing work here: vercel/next.js#64529

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

Successfully merging this pull request may close these issues.

6 participants