Skip to content
This repository was archived by the owner on Jan 28, 2025. It is now read-only.

Conversation

@mertyildiran
Copy link
Contributor

Closes #384

For testing, I've added the below lines to my serverless.yml file just like explained in CONTRIBUTING.md:

plugins:
  localPath: "/home/mertyildiran/Downloads/serverless-next.js/packages/serverless-plugin"
  modules:
    - index

and deployed to AWS CloudFront but I think it didn't use my fork. I also tried to directly edit from node_modules but I think it didn't take any effect too. Probably I'm doing something wrong in the local development process.

I'm opening this PR because the tests I've added show that it works. @danielcondemarin if you guide me for testing my fork on AWS CloudFront then I can provide you further info or fix if anything is wrong.

@danielcondemarin
Copy link
Contributor

Just realised I commented on the issue rather than here, for reference: #384 (comment)

@mertyildiran mertyildiran changed the title Preserve the request object properties by deep cloning Copy the properties of http.IncomingMessage.prototype to request object by deep cloning May 6, 2020
@mertyildiran
Copy link
Contributor Author

@danielcondemarin I just force pushed a commit and fixed the problems that you've mentioned in your comment.

I have deployed to AWS CloudFront and I can confirm that #384 is fixed. But because now we deep copying http.IncomingMessage.prototype into the request object, I'm not sure if this is a generalized solution or a special case to Passport.js.

@danielcondemarin
Copy link
Contributor

@danielcondemarin I just force pushed a commit and fixed the problems that you've mentioned in your comment.

I have deployed to AWS CloudFront and I can confirm that #384 is fixed. But because now we deep copying http.IncomingMessage.prototype into the request object, I'm not sure if this is a generalized solution or a special case to Passport.js.

This should be a solution not just to passport but also other libraries that monkey patch http.IncomingMessage’s prototype so I’m happy with it.
Just one more thing, do you mind applying the same fix and tests to https://github.com/danielcondemarin/serverless-next.js/tree/master/packages/apigw-lambda-compat?🙂

@mertyildiran
Copy link
Contributor Author

mertyildiran commented May 6, 2020

@danielcondemarin your link has an emoji in it. I didn't know that was possible. 😆

I force-pushed again.

@danielcondemarin danielcondemarin self-requested a review May 6, 2020 18:58
Copy link
Contributor

@danielcondemarin danielcondemarin left a comment

Choose a reason for hiding this comment

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

Looks great 💯

Let me know if you find any other problems getting https://github.com/hoangvvo/nextjs-mongodb-app working. I would love to have it in the examples/ folder, please PR it if you have time 🙏 .

@danielcondemarin
Copy link
Contributor

@danielcondemarin your link has an emoji in it. I didn't know that was possible. 😆

I force-pushed again.

Hahahah I didn't even realise I did it! 😄

@danielcondemarin danielcondemarin merged commit f101f56 into serverless-nextjs:master May 6, 2020
@danielcondemarin
Copy link
Contributor

I've now published serverless-next.js@1.11.3 with this 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError: req.logIn is not a function - Passport.js

2 participants