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

Universal user agent not working in next.js edge runtime #77

Closed
marcincodes opened this issue Aug 27, 2023 · 4 comments · Fixed by #78
Closed

Universal user agent not working in next.js edge runtime #77

marcincodes opened this issue Aug 27, 2023 · 4 comments · Fixed by #78

Comments

@marcincodes
Copy link

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

Universal user agent is used by Octokit but when importing it in next.js edge runtime fails. It seems that there provess.version is undefined. Following code works and resolve issue with using Ocotkit there, but it can be not sufficient. Feel free to let me know how this should be solved, and I'm happy to help on fix.

Here is the diff that solved my problem:

diff --git a/node_modules/universal-user-agent/dist-web/index.js b/node_modules/universal-user-agent/dist-web/index.js
index c550c02..a89b11f 100644
--- a/node_modules/universal-user-agent/dist-web/index.js
+++ b/node_modules/universal-user-agent/dist-web/index.js
@@ -3,7 +3,7 @@ function getUserAgent() {
         return navigator.userAgent;
     }
     if (typeof process === "object" && "version" in process) {
-        return `Node.js/${process.version.substr(1)} (${process.platform}; ${process.arch})`;
+        return `Node.js/${process.version?.substr(1)} (${process.platform}; ${process.arch})`;
     }
     return "<environment undetectable>";
 }

This issue body was partially generated by patch-package.

@gr2m
Copy link
Owner

gr2m commented Aug 28, 2023

very odd. I'd like to

  1. understand under what circumstances process.version is undefined
  2. add a comment to explain said circumstance
  3. fallback to something more useful than Node.js/undefined, maybe Node.js/<unknown version>

I'm open to a pull request for that. But right now I feel like this is a problem that should be fixed upstream in next.js, not here.

@marcincodes
Copy link
Author

Hi Gregor, sorry I bothered you.

I reported cross issue on next.js platform - vercel/next.js#54739
I agree that it should be resolved there, and it's a problem with Vercel edge runtime that is incompatible with nodejs.

To answer your questions and give you more context:
Vercel is making its "edge runtime" which is not compatible with nodejs. I think they implement some sort of process properties, and process global var itself but not the process.version
Here is an example of someone issue: vercel/next.js#33122

For some reason "version" in process is true but process.version is still undefined. Seems that value is assigned with undefined on purpose.

image

Solution should be different from my code - more strict check for process.version being a string.

Feel free to close this ticket. I find this issue and someone might stumble upon it too, made it just for visibility.

@gr2m
Copy link
Owner

gr2m commented Aug 29, 2023

thank you! If anything changes, please let me know!

@gr2m
Copy link
Owner

gr2m commented Nov 4, 2023

I ran into the problem now as well, I'll backport it to v6 as that is the one currently used by latest @octokit. Thanks again @marcincodes and @jaschaephraim

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 a pull request may close this issue.

2 participants