fix(defaultExport): fixed version string causing issues with Angular compiler#536
fix(defaultExport): fixed version string causing issues with Angular compiler#536nickiannone-fis wants to merge 1 commit into
Conversation
…compiler Co-authored-by: Copilot <copilot@github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the SDK versioning logic by updating how the version is imported from package.json in ts-embed.ts and mixpanel-service.ts. The review feedback identifies opportunities to optimize the production bundle size and improve maintainability by centralizing the version export in a single module, thereby avoiding the redundant import of the entire JSON object across multiple files.
| import { default as packageInfo } from '../package.json'; | ||
|
|
||
| const VERSION = packageInfo.version; |
There was a problem hiding this comment.
Importing the entire 'package.json' file as a default export can lead to the whole JSON object being included in the production bundle, which increases the SDK's footprint unnecessarily (including scripts, dependencies, and other metadata). Additionally, it exposes project metadata in the client-side code that isn't required at runtime.
Consider exporting the version string from this module so it can be reused in other parts of the SDK without re-importing the JSON file.
| import { default as packageInfo } from '../package.json'; | |
| const VERSION = packageInfo.version; | |
| import { default as packageInfo } from '../package.json'; | |
| export const VERSION = packageInfo.version; |
| import { uploadMixpanelEvent, MIXPANEL_EVENT } from '../mixpanel-service'; | ||
| import { processEventData, processAuthFailure } from '../utils/processData'; | ||
| import { version } from '../../package.json'; | ||
| import { default as packageInfo } from '../../package.json'; |
There was a problem hiding this comment.
To improve maintainability and reduce bundle size, consider importing the 'VERSION' constant from the mixpanel service instead of re-importing the 'package.json' file here. This avoids duplicating the extraction logic and ensures a single source of truth for the SDK version.
| import { default as packageInfo } from '../../package.json'; | |
| import { VERSION } from '../mixpanel-service'; |
| */ | ||
| export const THOUGHTSPOT_PARAM_PREFIX = 'ts-'; | ||
| const TS_EMBED_ID = '_thoughtspot-embed'; | ||
| const VERSION = packageInfo.version; |
There was a problem hiding this comment.
This local constant definition is redundant if the 'VERSION' is imported from the mixpanel service as suggested.
1 participant
Fixing issue described in #535
This converts the exports of the package.json file (for getting the build version number) into aliased default exports, fixing a build error encountered when using this in an Angular application.