Skip to content

Conversation

@Creskendoll
Copy link
Contributor

@Creskendoll Creskendoll commented May 4, 2023

As mentioned on #199 (review), trying to access process in a browser environment fails.

The availability of the value needs to be checked before accessing it.

You can test this easily by running process?.platform in the browser console. Instead of resolving to the expected undefined, it throws an error.

References

Relates to #199

@Creskendoll Creskendoll requested a review from a team as a code owner May 4, 2023 13:18
@Creskendoll Creskendoll requested a review from wraithgar May 4, 2023 15:22
@Creskendoll
Copy link
Contributor Author

Hey @wraithgar can you take another look when you have time, please? Test coverage should now pass.

@wraithgar
Copy link
Member

I can't seem to find an eslint rule that covers this, sadly. I'm kind of surprised by this and hope it's my lack of search skills. Filed npm/eslint-config#70 for looking into it later. We typically don't like to fix things like this w/o making sure it doesn't happen again (cf npm/node-semver#554)

@wraithgar wraithgar merged commit 090f64e into npm:main May 15, 2023
@github-actions github-actions bot mentioned this pull request May 15, 2023
@wraithgar wraithgar self-assigned this May 15, 2023
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.

2 participants