-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Native Module Docs need to be updated for RCT_EXPORT_MODULE #793
Comments
^-- Those are fantastic questions. I've been reading the PRs , especially ones coming from the FB team that say "Updates ...." Another point, i'd like to add to Colin's:
|
Do these changes resolve #579 in any way? |
Also, if FB has codemod regexes for the upgrade it'd be helpful to get those. |
We codemodded manually ¯_(ツ)_/¯ The old RCT_IMPORT() syntax for methods should still be supported for a while though, so no need to upgrade everything right away. |
👍 This is super annoying. I know we're still in very early development but I'm having to fix breaking changes for every single patch version that's released. Would be great if the docs were updated whenever breaking changes are introduced. |
@colinramsay - fixed in #801 |
Splendid, thanks! On Fri, Apr 10, 2015 at 10:38 PM, Brent Vatne notifications@github.com
|
@lwansbrough I understand that this is frustrating, and we'll try to do better, particularly with the docs updates. Release notes are problematic due to the way that fixes are landed internally and then pushed to github in batches. Again, we're actively working to improve that process. I would like to reassure you that there is some method to the madness though. We're aware that breaking changes are painful, and we'll do our best to a) avoid making them, and b) when we do make them, ensure that we break things in an obvious way, rather than having things silently fail. For example, in the change to module registration, we left the class scanning feature enabled in debug mode, purely so we could detect modules that have not yet been converted, and provide a useful error message. Similarly, for the RCT_EXPORT -> RCT_EXPORT_METHOD change, we've left in full support for the old system so that you can make the upgrades at your convenience instead of immediately breaking the apps. I'm pretty happy with the technical approach we've taken here, but we'll try to improve on the communication and documentation side too. |
The old approach doesn't seem to work, I tried it myself yesterday with the same result. http://stackoverflow.com/q/29552377/125680 On Sat, Apr 11, 2015 at 9:08 AM, Nick Lockwood notifications@github.com
|
Sorry if I was unclear: RTC_EXPORT_MODULE() is required, but RTC_EXPORT_METHOD() shouldn't be. These landed around the same time, but they're distinct changes. (i.e if the old RCT_EXPORT() approach of exporting methods isn't working, that's a bug, not deliberate). |
Previously we'd use RCT_EXPORT but it seems that there's been a change (9th Apr) that means we need to use RCT_EXPORT_MODULE on the class and RCT_EXPORT_METHOD for each method to be exported. The docs don't yet reflect this. It also raises two points:
The text was updated successfully, but these errors were encountered: