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

Fix color detection in edge runtime #56

Merged
merged 3 commits into from
Aug 16, 2023
Merged

Fix color detection in edge runtime #56

merged 3 commits into from
Aug 16, 2023

Conversation

alexeyraspopov
Copy link
Owner

@alexeyraspopov alexeyraspopov commented Aug 15, 2023

Per @brillout's comment here: #54 (comment) Edge Runtime has following constraints:

  • require is undefined
  • process.argv is undefined

Here I'm trying to fix this without blowing out the size of the package

picocolor.js size goes from 2594B to 2600B.

Closes #54

@alexeyraspopov alexeyraspopov marked this pull request as ready for review August 15, 2023 17:32
@alexeyraspopov
Copy link
Owner Author

@ai @brillout feel free to raise any concerns or suggestions before I merge and publish v1.0.1.

@brillout
Copy link

I ain't familiar with Vercel's Edge runtime; I don't know whether your version works as well.

FYI this is the fork I'm using: brillout/picocolors@efd8707...main — you may like some of it.

@alexeyraspopov alexeyraspopov merged commit b626148 into main Aug 16, 2023
8 checks passed
@alexeyraspopov alexeyraspopov deleted the edge-fix branch August 16, 2023 18:14
@hydRAnger
Copy link

@alexeyraspopov

thanks for the fix, I'd like to know do we have any plan to publish it on npm?
seems the current code is still outdated: https://www.npmjs.com/package/picocolors?activeTab=code

let tty = require("tty")

let argv = process.argv || [],
env = process.env
Copy link

@hydRAnger hydRAnger Oct 5, 2023

Choose a reason for hiding this comment

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

btw, shall we also handle scenarios when process is undefined? (like in the browser)

env = typeof process !== 'undefined' ? process.env : {};

or would you like me to make another PR?

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.

3 participants