call private ios pages API #69

RyanCommits · 2023-07-25T21:32:39Z

With the following JS code:

const startRecording = () => {
    let page;
    page = new FullStory.FSPage('Home', { tired: 'man', obj: { hmm: 'hmm' }, aaa: { hmm: 'hmm' } });
    page.start({ pageName: 'erp', Homes: 'erp', woman: 'wo', obj: 3, aaa: { pop: 'op' } });
    page.update({ updates: 'updates' });
    page.end();
  };

  const stopRecording = () => {
    let page;
    page = new FullStory.FSPage('Test');
    page.start();
    page.update({ updates: 'updates' });
    page.end();
  };

Old RN Architecture:
https://app.staging.fullstory.com/ui/KWH/session/4688480919289856:4741929862889472

New RN Architecture:
https://app.staging.fullstory.com/ui/KWH/session/4688480919289856:6558104800395264

* Fix TS types
* Fix immutable object merge issue (Old Architecture)
* Fix Java method declaration on Old Architecture

jwise

Hmm, Reece made a good point about compatibility with different versions. Some comments inline.

android/src/legacy/java/com/FullStoryModule.java

ios/FullStory.mm

jwise · 2023-07-26T23:35:49Z

ios/FullStory.mm

 	dispatch_async(dispatch_get_main_queue(), ^{
-		self->_page = [FS pageWithName:pageName properties:pageProperties];
+		[FS pageViewWithNonce:uuid name:pageName properties:pageProperties];


This comment in the related Android PR reminded me that this could be a problem if paired with a plugin that does not have pageViewWithNonce. We presumably either need to check the plugin version -- or we need to check this with something like:

Suggested change

[FS pageViewWithNonce:uuid name:pageName properties:pageProperties];

if (![FS respondsToSelector:@selector(pageViewWithNonce:name:properties:)]) {

...react native logging mechanism to complain...

} else {

[FS pageViewWithNonce:uuid name:pageName properties:pageProperties];

}

Added a check

ios/FullStory.mm

RyanCommits · 2023-08-26T02:30:38Z

android/src/main/java/com/fullstory/reactnative/FullStoryModuleImpl.java

@@ -35,7 +35,7 @@ public class FullStoryModuleImpl {
            updatePageProperties = null;
            endPage = null;
            Log.println(Log.ERROR, NAME, "Unable to access native FullStory pages API. Pages API will not function correctly. " +
-                    "Make sure that your plugin is at least version 1.38; if the issue persists, please contact FullStory Support.");
+                    "Make sure that your plugin is at least version 1.41; if the issue persists, please contact FullStory Support.");


Bumped this Android message to 1.41 since we edited the iOS private API's made available in 1.41. We want the recommended version for Pages API to be consistent.

This check and informative log message is Android-specific. We should have a similar check and log message for iOS.

The error message PagesAPIError in ios/FullStory.mm serves this purpose. Not sure if you missed it or if you mean something else?

- update legacy RN native module method declarations - replace System.out with android.util.Log - initialize reflected methods once - getUUID is async - fix page properties merge - add TS FSPage types

ReeceLaF

LGTM, but I'll let @jwise approve as its iOS.

martin-fs

Thanks for pointing out that iOS log message - I think it's because I was skimming for an "Unable to access pages" type of message and didn't see it. So, consider a couple of small string changes, but otherwise looks good!

martin-fs · 2023-09-06T17:42:21Z

ios/FullStory.mm

-	});
-}
+	if (![FS respondsToSelector:@selector(_updatePageWithNonce:properties:)]) {
+		RCTLogError(PagesAPIError, @"endPage");


I think "endPage" should be "updatePage" here.

martin-fs · 2023-09-06T17:44:08Z

ios/FullStory.mm

 }

+NSString *const PagesAPIError = @"Unable to call %@. Make sure that your plugin is at least version 1.41; if the issue persists, please contact FullStory Support.";


Could you add some verbiage similar to the Android log, like "Unable to access native FullStory pages API. Pages API will not function correctly. ". This will help the clarity of the log message and the consistency between platforms.

* add pages API to JS spec * add pages API to iOS * add Android pages API * Add Android pages implementation (#68) * Add Android pages implementation * updates on feedback - update legacy RN native module method declarations - replace System.out with android.util.Log - initialize reflected methods once - getUUID is async - fix page properties merge - add TS FSPage types * cleanup and fix turbomodule * remove private functions from export * remove page methods from types * add error message * generate UUID with Math.random() * remove async method * update method type to synchronous * update android log --------- Co-authored-by: Ryan Wang <[email protected]> * call private ios pages API (#69) * Add Android pages implementation * updates on feedback - update legacy RN native module method declarations - replace System.out with android.util.Log - initialize reflected methods once - getUUID is async - fix page properties merge - add TS FSPage types * cleanup and fix turbomodule * remove private functions from export * remove page methods from types * add error message * generate UUID with Math.random() * remove async method * update method type to synchronous * implement ios pages API * formatting * formatting * Fix rebase regression * Edit error message --------- Co-authored-by: Ryan Wang <[email protected]> * Improve Pages types and add testing (#76) Co-authored-by: Ryan Wang <[email protected]> * use uppercase UUIDs, exclude lib/ from jest * implement new uuid generator, update tests --------- Co-authored-by: Ryan Wang <[email protected]>

RyanCommits assigned jwise and ReeceLaF Jul 25, 2023

jwise reviewed Jul 26, 2023

View reviewed changes

RyanCommits force-pushed the ryanwang/android-pages-api branch 2 times, most recently from 5526414 to bcc384c Compare August 25, 2023 19:17

RyanCommits force-pushed the ryanwang/ios-pages-api branch from d5a88de to bd11bf6 Compare August 26, 2023 02:28

RyanCommits commented Aug 26, 2023

View reviewed changes

RyanCommits requested a review from jwise August 26, 2023 02:34

Base automatically changed from ryanwang/android-pages-api to ryanwang/add-pages-api August 31, 2023 18:13

rwang-fs added 12 commits August 31, 2023 14:17

Add Android pages implementation

31e1608

updates on feedback

362c27f

- update legacy RN native module method declarations - replace System.out with android.util.Log - initialize reflected methods once - getUUID is async - fix page properties merge - add TS FSPage types

cleanup and fix turbomodule

9cd037a

remove private functions from export

ba4541d

remove page methods from types

8db8ab7

add error message

57a0dc5

generate UUID with Math.random()

47852bd

remove async method

a2dad0d

update method type to synchronous

3bad749

implement ios pages API

0841ec4

formatting

526d292

formatting

984484e

RyanCommits force-pushed the ryanwang/ios-pages-api branch from 807b50d to 984484e Compare August 31, 2023 18:21

Fix rebase regression

4c41df2

RyanCommits force-pushed the ryanwang/ios-pages-api branch from a430578 to 4c41df2 Compare August 31, 2023 18:38

ReeceLaF reviewed Aug 31, 2023

View reviewed changes

martin-fs approved these changes Sep 6, 2023

View reviewed changes

Edit error message

5d88d47

RyanCommits merged commit c6ff20e into ryanwang/add-pages-api Sep 6, 2023

RyanCommits deleted the ryanwang/ios-pages-api branch September 6, 2023 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

call private ios pages API #69

call private ios pages API #69

RyanCommits commented Jul 25, 2023 •

edited

Loading

jwise left a comment

jwise Jul 26, 2023

RyanCommits Aug 26, 2023

RyanCommits Aug 26, 2023

martin-fs Sep 6, 2023

RyanCommits Sep 6, 2023

ReeceLaF left a comment

martin-fs left a comment

martin-fs Sep 6, 2023

martin-fs Sep 6, 2023

-		[FS pageViewWithNonce:uuid name:pageName properties:pageProperties];
+		if (![FS respondsToSelector:@selector(pageViewWithNonce:name:properties:)]) {
+			...react native logging mechanism to complain...
+		} else {
+			[FS pageViewWithNonce:uuid name:pageName properties:pageProperties];
+		}

		}

		NSString *const PagesAPIError = @"Unable to call %@. Make sure that your plugin is at least version 1.41; if the issue persists, please contact FullStory Support.";

call private ios pages API #69

call private ios pages API #69

Conversation

RyanCommits commented Jul 25, 2023 • edited Loading

jwise left a comment

Choose a reason for hiding this comment

jwise Jul 26, 2023

Choose a reason for hiding this comment

RyanCommits Aug 26, 2023

Choose a reason for hiding this comment

RyanCommits Aug 26, 2023

Choose a reason for hiding this comment

martin-fs Sep 6, 2023

Choose a reason for hiding this comment

RyanCommits Sep 6, 2023

Choose a reason for hiding this comment

ReeceLaF left a comment

Choose a reason for hiding this comment

martin-fs left a comment

Choose a reason for hiding this comment

martin-fs Sep 6, 2023

Choose a reason for hiding this comment

martin-fs Sep 6, 2023

Choose a reason for hiding this comment

RyanCommits commented Jul 25, 2023 •

edited

Loading