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

[Errors] Improve the error message shown when an unregistered callback is invoked #6300

Closed
ide opened this issue Mar 4, 2016 · 3 comments
Closed
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.

Comments

@ide
Copy link
Contributor

ide commented Mar 4, 2016

See #6286. RN expects exactly one callback to be called exactly once, after which point it unregisters the callbacks to free memory. We should print a nicer error message when an unregistered callback is invoked.

@ide ide added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. labels Mar 4, 2016
@agiron123
Copy link
Contributor

Is this issue still open? I see that the referenced issue is closed.
Anyways, I'm a first time contributor that would love to help out. Is there anything that I should keep in mind while I familiarize myself with the code base and begin to put together a PR with associated unit tests?

@ide
Copy link
Contributor Author

ide commented Mar 11, 2016

@agiron123 Hi, this issue is still open and thanks for offering to help. To answer your question, I would say the main thing to keep in mind is balancing the cost of maintaining the code with how valuable a feature is to everyone using React Native. For this issue specifically, I don't think that will be an issue -- a clearer error message in this case should be simple and strictly better than what we currently have.

agiron123 added a commit to agiron123/react-native that referenced this issue Mar 13, 2016
Fix for issue facebook#6300.
This fix adds a more descriptive error message for the error case when
more than one callback is registered to a native function.
ghost pushed a commit that referenced this issue Mar 23, 2016
Summary:Fix for issue #6300:
Motivation: When more than one callback is registered to a native module, the error message that a user receives is not indicative of what is really happening.
Closes #6436

Differential Revision: D3087551

Pulled By: tadeuzagallo

fb-gh-sync-id: 93c703348dc53b75c5b507edc71754680ab5c438
shipit-source-id: 93c703348dc53b75c5b507edc71754680ab5c438
@the4dpatrick
Copy link

Should this issue be closed since the PR was merged into master? fd2cf11

@ide ide closed this as completed Mar 30, 2016
zebulgar pushed a commit to nightingale/react-native that referenced this issue Jun 18, 2016
…acks.

Summary:Fix for issue facebook#6300:
Motivation: When more than one callback is registered to a native module, the error message that a user receives is not indicative of what is really happening.
Closes facebook#6436

Differential Revision: D3087551

Pulled By: tadeuzagallo

fb-gh-sync-id: 93c703348dc53b75c5b507edc71754680ab5c438
shipit-source-id: 93c703348dc53b75c5b507edc71754680ab5c438
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants