Skip to content
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

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

siyuniu-ms
Copy link
Contributor

No description provided.

return snippet;

let configString = JSON.stringify(snippetConfig);
let userSnippet = `!(function (cfg){${originSnippet}})(\n${configString}\n);`;

Check warning

Code scanning / CodeQL

Improper code sanitization Medium

Code construction depends on an
improperly sanitized value
.
ld?: number;
useXhr?: boolean;
crossOrigin?: string;
onInit?: any;
Copy link
Collaborator

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,
Copy link
Collaborator

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.

Copy link
Contributor Author

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!

@siyuniu-ms siyuniu-ms merged commit 977fcd5 into main Jun 21, 2024
7 checks passed
@siyuniu-ms siyuniu-ms added this to the webldr-1.2.0 milestone Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants