NOTE: this is pointing to a beta release of @fullstory/snippet this will be merged and become the first 2.0.0 beta release.
There are a few big things I did with this PR:
Convert to typescript:
In order to maintain type consistency in the FS api between releases, the @fullstory/snippet package now exports types for the Public api. It was just easier to get rollup to properly generate the types file by converting everything. Bonus: I wrote some tests around making sure the types are working 💯 .
Import signature will change:
Because the api object is a function, we can no longer wildcard import everything. As a result, we have two options:
The one implemented here:
import FullStory, { init } from '@fullstory/browser';
The one Rollup doesn't complain about:
import { FullStory, init } from '@fullstory/browser';
(see open questions for more details)
Maintain support for legacy api and V2 api:
There was initially a discussion around fully converting over to the v2 api. I decided since snippet v2 is backward compatible with the legacy api, it might ease adoption to make the SDK backward compatible as well.
Open questions:
What is our browser support?
I use Object.assign which has support everywhere but IE11. I could just as easily not use Object.assign (see comment on the code block itself).
What should the import signature look like.
Initially we discussed a signature like the one implemented.
import FullStory, { init } from '@fullstory/browser';
However, I think rollup has a valid complaint about this behavior and its compatibility with commonJS modules. (details here) I think it would only be an issue in a few cases, but to be more widely adoptable, we should consider moving to named exports:
import { FullStory, init } from '@fullstory/browser';
This brings me to the next open question
What should we name the export if we use named exports:
I think the logical choices are:
FullStory: The naming matches the current documentation in this SDK. Might be easier for adoption.
FS: Matches all examples in the developer documentation. The only issue being there is technically also a global variable called FS which is similar but slightly different.
Should we update node on CI?
Looks like I tested the build locally with node 16 which passes, but CI has node 12 🤷. Does it make sense to bump CI? How do we do this? 😂
Migration notes?
We should probably add some migration notes (like in the README). Where do we think that makes sense?
There are a few big things I did with this PR:
Convert to typescript:
In order to maintain type consistency in the FS api between releases, the
@fullstory/snippetpackage now exports types for the Public api. It was just easier to get rollup to properly generate the types file by converting everything. Bonus: I wrote some tests around making sure the types are working 💯 .Import signature will change:
Because the api object is a function, we can no longer wildcard import everything. As a result, we have two options: The one implemented here:
The one Rollup doesn't complain about:
(see open questions for more details)
Maintain support for legacy api and V2 api:
There was initially a discussion around fully converting over to the v2 api. I decided since snippet v2 is backward compatible with the legacy api, it might ease adoption to make the SDK backward compatible as well.
Open questions:
What is our browser support?
I use
Object.assignwhich has support everywhere but IE11. I could just as easily not useObject.assign(see comment on the code block itself).What should the import signature look like.
Initially we discussed a signature like the one implemented.
However, I think rollup has a valid complaint about this behavior and its compatibility with commonJS modules. (details here) I think it would only be an issue in a few cases, but to be more widely adoptable, we should consider moving to named exports:
This brings me to the next open question
What should we name the export if we use named exports:
I think the logical choices are:
FullStory: The naming matches the current documentation in this SDK. Might be easier for adoption.FS: Matches all examples in the developer documentation. The only issue being there is technically also a global variable calledFSwhich is similar but slightly different.Should we update node on CI?
Looks like I tested the build locally with node 16 which passes, but CI has node 12 🤷. Does it make sense to bump CI? How do we do this? 😂
Migration notes?
We should probably add some migration notes (like in the README). Where do we think that makes sense?