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

Add better guard against location definition #154

Merged

Conversation

acostalima
Copy link
Contributor

@acostalima acostalima commented Jan 29, 2021

Recently, I've run across a situation in a React Native environment where location was defined but location.host and location.protocol were not. The changes I propose herein defend the code better in such situation.

I've also took the opportunity to replace self with globalThis using globalthis shim.

package.json Outdated Show resolved Hide resolved
@acostalima acostalima force-pushed the fix/better-guard-against-location branch 8 times, most recently from dd7102e to 005b9f3 Compare March 2, 2021 15:56
@acostalima acostalima force-pushed the fix/better-guard-against-location branch from 005b9f3 to be8c037 Compare March 2, 2021 16:02
@acostalima
Copy link
Contributor Author

acostalima commented Mar 2, 2021

@hugomrdias I'm not sure whether I've messed up yarn.lock during conflict resolution. Do you mind checking on your end, please?

@hugomrdias
Copy link
Owner

can we add the RN test runner to the ci ?

@acostalima
Copy link
Contributor Author

acostalima commented Mar 2, 2021

can we add the RN test runner to the ci ?

I thought about this today as well, haha. Sure, I think it makes sense!
Shall we have a separate test suite to run in RN?

@acostalima
Copy link
Contributor Author

@hugomrdias can we merge this anyway to unblock ipfs/js-ipfs-utils#91? I'll add RN test runner tomorrow in a separate PR.

@hugomrdias hugomrdias merged commit 9dc71a9 into hugomrdias:master Mar 3, 2021
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