-
Notifications
You must be signed in to change notification settings - Fork 243
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
[main][snippet] prepare new snippet release, add support for more config #2365
Conversation
return snippet; | ||
|
||
let configString = JSON.stringify(snippetConfig); | ||
let userSnippet = `!(function (cfg){${originSnippet}})(\n${configString}\n);`; |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
ld?: number; | ||
useXhr?: boolean; | ||
crossOrigin?: string; | ||
onInit?: any; |
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.
onInit: should be defined as a function -- although I'm not sure that the JSON.stringify() is going to do what we want
name: config.name ? config.name : "appInsights", | ||
ld: config.ld, | ||
useXhr: config.useXhr, | ||
onInit: config.onInit, |
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.
Can you write a quick test to see if you pass in a function whether it will actually get called?
I don't think it will, as I think this process is just going to populate with a text version of the function....
If it doesn't work, lets just remove it from this version as we are going to have to do something tricky to handle / pass a Promise through this for the same reason as why I think the function isn't going to work.
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.
it doesn't work, I will remove onInit for this version. Thanks!
No description provided.