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

Nextjs improvements #99

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Nextjs improvements #99

merged 1 commit into from
Jul 18, 2024

Conversation

theoephraim
Copy link
Contributor

@theoephraim theoephraim commented Jun 27, 2024

NextJS integration now fully working, and tested with vercel deployments.

Security features (log redaction, leak detection, http interception) now all working in all scenarios (edge+node runtimes, app + pages router, api routes + pages, etc etc...) The matrix is very large and nextjs handles everything slightly differently in each case, so I'm sure we'll find some bugs with some real world usage.

Along the way, we refactored some things, running all leak detection through a single helper, and adding a service setting to enable/disable.

Also had to temporarily make the http interception only work with fetch, because the other interceptors break edge builds. Will need to figure out a way to make 2 builds, but it's all a bit awkward so will come back to it.

Copy link

netlify bot commented Jun 27, 2024

Deploy Preview for dmno ready!

Name Link
🔨 Latest commit 93f1e47
🔍 Latest deploy log https://app.netlify.com/sites/dmno/deploys/6699611ca5ab460008f2b8d9
😎 Deploy Preview https://deploy-preview-99--dmno.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jun 27, 2024

Deploy Preview for signup-api canceled.

Name Link
🔨 Latest commit 93f1e47
🔍 Latest deploy log https://app.netlify.com/sites/signup-api/deploys/6699611c40a6170008e84675

@theoephraim theoephraim force-pushed the nextjs-improvements branch 4 times, most recently from 9510d05 to db38a38 Compare July 17, 2024 04:45
@theoephraim theoephraim force-pushed the nextjs-improvements branch 2 times, most recently from 16430aa to 5208572 Compare July 17, 2024 18:52
Copy link
Contributor

@philmillman philmillman left a comment

Choose a reason for hiding this comment

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

few nits and a non-blocking suggestion, :shipit:


:::note
In general, these features should _just work_ but the matrix of app/pages router, node/edge, pages/api, and hosting platforms means that things are quite complicated. If you notice any issues, please <BugReportLink label='integrations/nextjs'>report them to us on GitHub</BugReportLink>!
:::
Copy link
Contributor

Choose a reason for hiding this comment

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

love this

next.js integration now working correctly on vercel and with various security features enabled in all the modes app/pages, node/edge, page/api, etc

additional refactoring and cleanup of related code
@theoephraim theoephraim merged commit f15eae3 into main Jul 18, 2024
7 of 8 checks passed
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.

None yet

2 participants