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 3 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.