Skip to content

Conversation

divyanshu013
Copy link
Member

No description provided.

@divyanshu013 divyanshu013 self-assigned this Oct 16, 2021
@divyanshu013 divyanshu013 merged commit 722fd1d into master Oct 16, 2021
@divyanshu013 divyanshu013 deleted the div/remove-old-api branch October 16, 2021 10:50
@efstathiosntonas
Copy link
Contributor

efstathiosntonas commented Oct 20, 2021

Hi @divyanshu013, I’ve noticed that you removed optional chaining on subscription?.remove(), please keep in mind that in some rare occasions subscription could be undefined thus crashing the app.

@divyanshu013
Copy link
Member Author

Oh didn't know about this, was following the docs. I tested this out on my device as well but didn't see any issues. Have you seen this in some particular case?

@efstathiosntonas
Copy link
Contributor

Unfortunately I don’t know how to reproduce it on my app, it seems it’s totally random and it happens from time to time.

I’ve seen other popular modules using an optional chaining for that reason, eg: https://github.com/ammarahm-ed/react-native-actions-sheet/blob/c2a266e0385b373f06332d44684be6399fe6232a/src/index.tsx#L560

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