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.

Sign up for GitHub

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

Jump to bottom

has-flag.js - seems to assume cli use, causing issue during synchronous module loading #41

Closed
YeeP79 opened this issue Apr 12, 2023 · 4 comments · Fixed by #44
Closed

has-flag.js - seems to assume cli use, causing issue during synchronous module loading #41

YeeP79 opened this issue Apr 12, 2023 · 4 comments · Fixed by #44

Comments

@YeeP79
Copy link

YeeP79 commented Apr 12, 2023 •
edited

The issue looks to be here:
https://github.com/DABH/colors.js/blob/master/lib/system/has-flag.js#L28

Node is just loading all imports into memory like it always does, and an error shows up:
image

Some debugging ahead of time will show that neither argv nor process.argv are defined.
image

Looking for any suggestions as to what can cause this. As the dependency chain comes from winston, I would image you would be seeing more issues raised if it was a straight up bug. Thank you
@valueof
Copy link

valueof commented Apr 22, 2023

I was just dealing with a similar issue. In our case, we have a pretty generic Next app. We've decided to add winston for structured logging and this error pops up only when logging is being added to Next Middleware (middleware.ts). I believe this happens because middleware uses Edge Runtime which doesn't have process.argv.

For now, I've moved logging code out of middleware. If I initialize the logger anywhere in server-side pages/api it seems to work fine.

@axelvaindal
Copy link

I just had the same issue with Next 13 using app directory.

As stated by @valueof, running winston logger inside middleware breaks. I've also moved my logger out for now.

@axelvaindal axelvaindal mentioned this issue May 27, 2023
Closed
1 task
@copini copini mentioned this issue Jul 6, 2023
Merged
@copini
Copy link
Contributor

copini commented Jul 6, 2023

Ran into the same issue using Next 13 (middleware + app directory). Was able to fix it locally using yarn patch, applying the same minor change as in PR #44.

@DABH DABH closed this as completed in #44 Jul 10, 2023
@DABH
Copy link
Owner

DABH commented Jul 10, 2023

This has been closed and released in v1.6.0. Thanks all!

@jakeleventhal jakeleventhal mentioned this issue Feb 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support environments without process.argv copini/colors.js
5 participants
@valueof @DABH @copini @YeeP79 @axelvaindal