-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add load options #66
Add load options #66
Conversation
@mzogheib Thank you so much for your contribution and I'm sorry we didn't have time to take a look at it earlier!
Would you mind adding a screenshot showing the Cheers |
if (typeof settings.load !== 'boolean') { | ||
// eslint-disable-next-line no-restricted-globals | ||
var loadOptions = JSON.stringify(settings.load); | ||
return 'analytics.load("' + settings.apiKey + '", ' + loadOptions + ');'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to be fully compliant with the typescript definitions added on #55 it'd be great if we could change the type of load?: boolean
to something like
type LoadOptions = boolean | { ... }
// ...
load?: LoadOptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @juliofarah and no problem! I'll provide this soon and address your other comments too. |
Codecov Report
@@ Coverage Diff @@
## master #66 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 33 36 +3
Branches 11 12 +1
=========================================
+ Hits 33 36 +3
Continue to review full report at Codecov.
|
Reference: #65
Usage