fullstorydev / fullstory-browser-sdk

Official FullStory SDK for JavaScript, for web browsers
MIT License
55 stars 17 forks source link

Add iFrameOption to snippet #53

Closed van-fs closed 4 years ago

van-fs commented 4 years ago

This PR fixes #1 and adds control over the snippet's iFrame recording options as seen in Can FullStory capture content that is presented in iframes.

Naming conventions

I opted for the following properties: RecordCrossDomainIFrame RecordOnlyIFrame

The Record prefix better specifies what the property controls. As in RecordCrossDomainIFrame records cross domain IFrames. The other RecordOnlyIFrame differs from the previously proposed OnlyInHostIFrame. To me, it's not clear only in host iframe means, and the underlying _fs_is_outer_script value is even more opaque. So I chose an option that implies only the iFrame will be recorded. Let me know if that's still not simple enough.

Readme

I added an options table to describe these new options as well as a few others. I've omitted the script and host options intentionally as these are more advanced needs, and I don't want devs to shoot themselves in the foot.

Code

There are two changes, one to the declaration file and the other to the JavaScript.

The following typescript enum:

export enum IFrameOption {
  RecordCrossDomainIFrame = '_fs_run_in_iframe',
  RecordOnlyIFrame = '_fs_is_outer_script',
}

normally gets transpiled to:

var IFrameOption;
(function (IFrameOption) {
    IFrameOption["RecordCrossDomainIFrame"] = "_fs_run_in_iframe";
    IFrameOption["RecordOnlyIFrame"] = "_fs_is_outer_script";
})(IFrameOption = exports.IFrameOption || (exports.IFrameOption = {}));

This is a bit obtuse (not to mention the lint errors it generates), and I simply re-wrote that as an exported object:

export const IFrameOption = {
  RecordCrossDomainIFrame: '_fs_run_in_iframe',
  RecordOnlyIFrame: '_fs_is_outer_script',
};
patrick-fs commented 4 years ago

I don't think this requires a change to the snippet. You can decorate the window object with non-snippet _fs attributes in the SDK init function.

patrick-fs commented 4 years ago

Hey Van, this is great! Thank you. I've added some comments for your review in the thread.

patrick-fs commented 4 years ago

Hey @van-fs , I just smoked tested in a couple of apps and this is looking great. Let's talk about enum names a little more :) I've left comments in the thread.

van-fs commented 4 years ago

Closing in favor of updated design, see PR 54