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

React Native Support #227

Merged

Conversation

colincasey
Copy link
Contributor

This PR fixes #222 by:

  • introducing a fallback for the util module code paths used as indicated in this comment
  • adding a dependency on url-parse to act as a polyfill for the usage of require('url').parse
  • configuring eslint to error on no-restricted-modules which will not be present in native environments

This PR fixes salesforce#222 by:
 - introducing a fallback for the `util.*` code paths used as indicated in [this comment](salesforce#222 (comment))
 - adding a dependency on `url-parse` to act as a polyfill for the usage of `require('url').parse`
This PR fixes salesforce#222 by:
 - introducing a fallback for the `util.*` code paths used as indicated in [this comment](salesforce#222 (comment))
 - adding a dependency on `url-parse` to act as a polyfill for the usage of `require('url').parse`
 - configuring eslint to error on `no-restricted-modules` which will not be present in native environments
Copy link
Contributor

@wjhsf wjhsf left a comment

Choose a reason for hiding this comment

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

With this change, the output of inspect for any MemoryCookieStore is always { idx: }. I've commented with a few ways that we might be able to avoid losing information.

test/node_util_fallback_test.js Outdated Show resolved Hide resolved
lib/node-util.js Outdated Show resolved Hide resolved
lib/node-util.js Outdated Show resolved Hide resolved
lib/memstore.js Show resolved Hide resolved
lib/node-util.js Outdated Show resolved Hide resolved
lib/cookie.js Outdated Show resolved Hide resolved
lib/node-util.js Outdated Show resolved Hide resolved
colincasey and others added 3 commits November 24, 2021 22:13
Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
Implementation for the fallback inspect function so that it produces equivalent output to the original inspect behavior.
Implementation for the fallback inspect function so that it produces equivalent output to the original inspect behavior.
lib/nodeUtil.js Outdated Show resolved Hide resolved
Implementation for the fallback inspect function so that it produces equivalent output to the original inspect behavior.
@colincasey colincasey requested a review from wjhsf December 2, 2021 13:41
Implementation for the fallback inspect function so that it produces equivalent output to the original inspect behavior.
wjhsf
wjhsf previously approved these changes Dec 2, 2021
@awaterma
Copy link
Member

I'll try and find some time this week to do the review; thank you so much for the submission!

Copy link
Member

@awaterma awaterma left a comment

Choose a reason for hiding this comment

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

Just had a question about one of the unit tests ...

test/node_util_fallback_test.js Show resolved Hide resolved
wjhsf
wjhsf previously approved these changes Jan 12, 2022
test/node_util_fallback_test.js Outdated Show resolved Hide resolved
Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
awaterma
awaterma previously approved these changes Jan 13, 2022
Copy link
Member

@awaterma awaterma left a comment

Choose a reason for hiding this comment

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

LGTM.

wjhsf
wjhsf previously approved these changes Jan 13, 2022
@pe-johndpope
Copy link

@ruoho-sfdc - please review

@awaterma
Copy link
Member

awaterma commented Mar 4, 2022

I think this is ready for merge @colincasey. Nice work!

@awaterma awaterma dismissed stale reviews from wjhsf and themself via 8ef9e80 March 4, 2022 22:29
Copy link
Member

@awaterma awaterma left a comment

Choose a reason for hiding this comment

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

Approved -- merged with latest update of universalify from Snyk.

@awaterma awaterma requested a review from wjhsf March 4, 2022 22:31
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.

[RFC] Support for React Native
4 participants