Skip to content

Conversation

@gtamanaha
Copy link
Contributor

Background

As part of the ongoing effort to migrate the mParticle Web SDK codebase to TypeScript, the logger.js module needed to be converted to TypeScript to improve type safety, code maintainability, and developer experience. The logger is a core component used throughout the SDK for handling different log levels (none, error, warning, verbose) and custom logger implementations.

What Has Changed

  • Migrated src/logger.js to src/logger.ts
    • Converted the Logger class to TypeScript with proper type annotations
    • All methods now have explicit parameter and return types
  • Added Type Definitions
    • Created ILoggerConfig type that picks logLevel and logger properties from SDKInitConfig
    • Created IConsoleLogger type that picks error, warning, and verbose methods from SDKLoggerApi and wraps them in Partial<> to make methods optional
    • Both types reuse existing SDK interfaces to maintain consistency
  • Enhanced Type Safety
    • Used Partial<Pick<>> utility types to ensure IConsoleLogger matches the optional method signature pattern from MPConfiguration.Logger
    • Properly typed the logLevel property using the LogLevelType enum constants (LogLevelType.Warning, LogLevelType.None, etc.) instead of string literals
    • Maintained the safe hasOwnProperty check in the constructor to handle edge cases with falsy logger values
  • Exported ConsoleLogger
    • Made ConsoleLogger class exportable for potential reuse in tests and other modules
  • Updated Test Files
    • Modified test/src/tests-audience-manager.ts and test/src/tests-batchUploader.ts to work with the new TypeScript logger implementation
  • Built Distribution Files
    • Regenerated dist/mparticle.common.js, dist/mparticle.esm.js, and dist/mparticle.js to include the TypeScript-compiled logger

Screenshots/Video

  • {Include any screenshots or video demonstrating the new feature or fix, if applicable}

Checklist

  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have tested this locally.

Additional Notes

  • {Any additional information or context relevant to this PR}

Reference Issue (For employees only. Ignore if you are an outside contributor)

@gtamanaha gtamanaha marked this pull request as ready for review November 21, 2025 15:56
@gtamanaha gtamanaha self-assigned this Nov 21, 2025
@gtamanaha gtamanaha requested a review from rmi22186 November 21, 2025 16:41
@gtamanaha gtamanaha requested a review from mattbodle November 24, 2025 12:42
Copy link
Member

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nits

@sonarqubecloud
Copy link

@gtamanaha gtamanaha requested a review from rmi22186 November 24, 2025 19:50
@rmi22186
Copy link
Member

LGTM. I modified the PR to go into blackout2025 just in case we have a hotfix that we need.

@rmi22186 rmi22186 changed the base branch from development to blackout2025 November 24, 2025 21:58
@gtamanaha gtamanaha merged commit 4cc5aa8 into blackout2025 Nov 25, 2025
28 of 32 checks passed
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.

4 participants