Address issue with the AppInsightsExtCore using the wrong version number#2718
Address issue with the AppInsightsExtCore using the wrong version number#2718
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -1,6 +1,9 @@ | |||
| const { default: replace } = require("@rollup/plugin-replace"); | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
To fix the problem, we should remove the unused variable by deleting the destructuring import of replace from @rollup/plugin-replace. This avoids confusion and a wasted require call at runtime. Since replace is not used anywhere in the provided gruntfile.js code, removing this line does not change existing functionality.
Concretely, in gruntfile.js, delete line 1:
- Remove:
const { default: replace } = require("@rollup/plugin-replace");
No additional methods, imports, or definitions are required, because nothing else depends on this symbol.
| @@ -1,4 +1,3 @@ | ||
| const { default: replace } = require("@rollup/plugin-replace"); | ||
|
|
||
| module.exports = function (grunt) { | ||
|
|
There was a problem hiding this comment.
Pull request overview
This PR fixes the version value reported by AppInsightsExtCore (ext.sdk.ver) by separating the “extended/1DS” version from the core package version, aligning 1DS-Web-JS-* with the expected 1DS major version.
Changes:
- Introduces
ExtVersion/ExtFullVersionStringfor the extended SDK and updatesAppInsightsExtCoreto use it forext.sdk.ver. - Updates Grunt version placeholder replacement to support
#extVersion#(derived as core major + 1) and adds unit tests validating the intended ownership ofext.sdk.ver. - Updates unit tests to validate the new version constants and behavior boundaries between
AppInsightsCorevsAppInsightsExtCore.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/AppInsightsCore/src/ext/extUtils.ts | Adds ExtVersion / ExtFullVersionString and keeps legacy Version / FullVersionString as deprecated aliases. |
| shared/AppInsightsCore/src/ext/AppInsightsExtCore.ts | Switches ext.sdk.ver to use ExtFullVersionString. |
| shared/AppInsightsCore/src/index.ts | Exports the new extended-version constants from the public entrypoint. |
| gruntfile.js | Adds support for replacing/restoring #extVersion# during builds (core major+1 mapping). |
| shared/AppInsightsCore/Tests/Unit/src/ext/CoreTest.ts | Updates assertions to validate ext.sdk.ver against ExtFullVersionString and the legacy alias. |
| shared/AppInsightsCore/Tests/Unit/src/ai/ApplicationInsightsCore.Tests.ts | Adds tests ensuring AppInsightsCore.track() does not populate/alter ext.sdk.ver. |
| common/config/rush/npm-shrinkwrap.json | Removes the node_modules/fsevents entry (lockfile churn). |
Files not reviewed (1)
- common/config/rush/npm-shrinkwrap.json: Language not supported
shared/AppInsightsCore/Tests/Unit/src/ai/ApplicationInsightsCore.Tests.ts
Show resolved
Hide resolved
No description provided.