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

#760 - update EventSource definition for streaming operations in React Native #761

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

hunterpetersen
Copy link
Contributor

Updated call_builder.ts to include another condition for defining EventSource.

The addition of the second else if clause allows the correct definition to be applied in React-Native environments under the else condition.

Previous definition:

if (anyGlobal.EventSource) {
  EventSource = anyGlobal.EventSource;
} else if (isNode) {
  /* tslint:disable-next-line:no-var-requires */
  EventSource = require("eventsource");
} else (anyGlobal.window.EventSource) { 
  // RN environment does not trigger anyGlobal.EventSource or isNode
  // anyGlobal.window.EventSource is undefined here
  EventSource = anyGlobal.window.EventSource;
}

Additional clause preserving anyGlobal.window.EventSource conditions while correctly assigning EventSource in RN:

else if (anyGlobal.window.EventSource) { 
  EventSource = anyGlobal.window.EventSource;
} else {
  // require("eventsource") for React Native env
  /* tslint:disable-next-line:no-var-requires */
  EventSource = require("eventsource");
}

Copy link
Contributor

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

Seems good to me 👍 a better fallback than undefined, after all.

@Shaptic
Copy link
Contributor

Shaptic commented Apr 11, 2022

@hunterpetersen unfortunately, we require all commits to be signed before they can be merged into the repo. Would you mind setting that up here? If you do, I can also cherry-pick your commits and sign them as my own in a separate branch.

@hunterpetersen
Copy link
Contributor Author

hunterpetersen commented Apr 11, 2022

@Shaptic made a new commit, should be signed / verified. Thanks!

@Shaptic
Copy link
Contributor

Shaptic commented Apr 12, 2022

Bahh sorry, I thought we were good but you still have the unsigned ones in the history @hunterpetersen

You can do something like

git rebase -i HEAD~5

then put pick on the first line and fixup on the others, then

git commit --amend -S

to re-sign the latest commit, and finally

git push -f

to force-push and overwrite the existing ones.

@hunterpetersen
Copy link
Contributor Author

hunterpetersen commented Apr 12, 2022

@Shaptic no problem, should be good now.

@Shaptic Shaptic merged commit 76bec21 into stellar:master Apr 12, 2022
@Shaptic
Copy link
Contributor

Shaptic commented Apr 12, 2022

Thank you for your 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.

2 participants