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

feat!: remove process global to work outside of node #129

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

styfle
Copy link
Contributor

@styfle styfle commented Feb 6, 2024

In order to support more JS runtimes besides Node.js, this PR drops the check for process global.

The process global was only used to improve the error message for Node.js less than 10 (released April 2018).

This could be considered a breaking change since the error message will change, however this package already specifies package.json engines.node with >=10, so perhaps no one can even run it on older versions of Node.js without an install error.

"node": ">=10"

@coveralls
Copy link

coveralls commented Feb 6, 2024

Coverage Status

coverage: 92.382% (+0.03%) from 92.355%
when pulling 3b2726c on styfle:remove-process-global
into 5214db4 on micromatch:master.

@styfle
Copy link
Contributor Author

styfle commented Feb 6, 2024

I realize now that typeof process !== 'undefined' should have caught this 🤦‍♂️

My use case is actually babel attempting to read process as seen in vercel/next.js#61116

I think this PR is still useful though since the default value assumed false for non-Node.js runtimes and my PR assumes true 😃

@jonschlinkert
Copy link
Member

@styfle thanks for the PR. I agree with all of your points. I'll get this merged in an probably publish a major asap. Thanks!

@styfle
Copy link
Contributor Author

styfle commented Feb 7, 2024

@jonschlinkert Thanks for the quick response! 🎉

@jonschlinkert jonschlinkert merged commit c0f9c55 into micromatch:master Feb 8, 2024
15 checks passed
@styfle styfle deleted the remove-process-global branch February 8, 2024 01:15
@jonschlinkert
Copy link
Member

Done, I just published v4.0.

If anyone wants to do a PR in micromatch to upgrade picomatch to latest, I'd be happy to accept.

@styfle
Copy link
Contributor Author

styfle commented Feb 8, 2024

Great! I created a micromatch PR here:

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