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

Revert "perf: use node: prefix to bypass require.cache call for builtins (#644)" #660

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

mcollina
Copy link
Member

@mcollina mcollina commented Nov 3, 2023

This reverts commit 4073e5b.

Signed-off-by: Matteo Collina [email protected]

Fixes #659

…ltins (#644)"

This reverts commit 4073e5b.

Signed-off-by: Matteo Collina <[email protected]>

Fixes #659
@mcollina mcollina requested a review from Fdawgs November 3, 2023 09:51
@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 3, 2023

IMO
Why do we not reach out also to next.js and report this? Maybe they need to fix resolving node packages? I mean, there is probably other cases which are independent from us, which break also.

@mcollina
Copy link
Member Author

mcollina commented Nov 3, 2023

Agreed. I guess it's simpler to revert for now and reassess for them.
I think we could avoid them for libraries and keep them in Fastify proper.

@inlet
Copy link

inlet commented Nov 3, 2023

Thanks @mcollina for addressing this issue so quickly! Looking forward to a new working release 🙏

@Fdawgs
Copy link
Member

Fdawgs commented Nov 3, 2023

Bloody front end developers ruining everything. 😂

@inlet
Copy link

inlet commented Nov 3, 2023

@Fdawgs haha! Sorry mate

@mcollina mcollina merged commit 76603c5 into master Nov 3, 2023
21 checks passed
@Uzlopak Uzlopak deleted the revert-nodecolon branch November 3, 2023 10:29
@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 3, 2023

Who is gonna report it to next.js?

Maybe @inlet?

@inlet
Copy link

inlet commented Nov 3, 2023

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.

node:crypto issues in Next.js
4 participants