Skip to content

feat: improve MSAL module version checking to be more permissive #3376

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: improve MSAL module version checking to be more permissive #3376

wants to merge 6 commits into from
+860 −45

Conversation

odinr
Copy link
Collaborator

@odinr odinr commented Sep 15, 2025 •
edited
Loading

Why

This PR introduces improvements to the MSAL module version checking to be more permissive for minor and patch versions.

What is the current behavior?
The MSAL module currently has strict version checking that can cause failures when different parts of an application use slightly different minor or patch versions of the MSAL module.

What is the new behavior?

  • Version checking is now more permissive for minor and patch versions
  • Only major version incompatibilities will block execution
  • Minor version differences will show warnings but allow execution to continue
  • Patch version differences are completely ignored
  • Improved error messages and warnings for better developer experience

Does this PR introduce a breaking change?
No, this change maintains backward compatibility with existing MSAL module configurations.

Other information?
This refactors the version checking logic into a dedicated versioning module with comprehensive test coverage.

Testing

Review Testing Steps

  1. Checkout the PR: gh pr checkout 3376
  2. Test app-react-msal cookbook: 
    pnpm --filter @equinor/fusion-framework-cookbook-app-react-msal dev
  3. Verify no MSAL version errors in console - The app should run without version-related errors

closes: #3375

Check off the following:

  • Confirm that I checked changes to branch which I am merging into.

    • I have validated included files
    • My code does not generate new linting warnings
    • My PR is not a duplicate, check existing pr`s

  • Confirm that the I have completed the self-review checklist.

  • Confirm that my changes meet our code of conduct.

@odinr
feat: add spec for MSAL module version checking improvement
9852b70 
- Add specification for more permissive version checking
- Define user scenarios for major/minor/patch version handling
- Include functional requirements for version compatibility
- Reference GitHub issue #3375

Closes #3375
@odinr
feat: improve MSAL module version checking to be more permissive
71d57f4 
- Refactor version checking logic into dedicated versioning module
- Make version checking more permissive for minor and patch versions
- Add comprehensive test coverage for version resolution
- Improve error messages and warnings for version mismatches
- Update spec to reflect new branch naming convention

Closes #3375
@Copilot Copilot AI review requested due to automatic review settings September 15, 2025 10:34
@odinr odinr requested a review from a team as a code owner September 15, 2025 10:34
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Sep 15, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 59ae6a4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 22 packages
Name Type
@equinor/fusion-framework-module-msal Minor
@equinor/fusion-framework-app Patch
@equinor/fusion-framework Patch
@equinor/fusion-framework-module-http Patch
@equinor/fusion-framework-module-signalr Major
@equinor/fusion-framework-module-telemetry Patch
@equinor/fusion-framework-react-app Major
@equinor/fusion-framework-react Patch
@equinor/fusion-framework-legacy-interopt Major
@equinor/fusion-framework-widget Patch
poc-portal Patch
@equinor/fusion-framework-module-service-discovery Patch
@equinor/fusion-framework-module-widget Patch
@equinor/fusion-framework-react-module-http Patch
@equinor/fusion-framework-cookbook-app-react-feature-flag Patch
@equinor/fusion-framework-react-module-signalr Patch
@equinor/fusion-framework-cookbook-app-react-ag-grid Patch
@equinor/fusion-framework-cookbook-app-react-context-custom-error Patch
@equinor/fusion-framework-cookbook-app-react-context Patch
@equinor/fusion-framework-react-components-bookmark Patch
@equinor/fusion-framework-react-components-people-provider Patch
@equinor/fusion-framework-react-module-bookmark Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added 📚 documentation Improvements or additions to documentation 🚀 feature New feature or request 🧬 Modules labels Sep 15, 2025
Copilot
Copilot AI reviewed Sep 15, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors MSAL module version checking to be more permissive for minor and patch versions while maintaining strict checking for major version incompatibilities.

  • Version checking now allows minor version differences with warnings instead of failures
  • Patch version differences are completely ignored for maximum compatibility
  • Comprehensive versioning module with dedicated error handling and test coverage

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
specs/0002-improve-msal-versioning/spec.md Feature specification documenting version checking requirements and acceptance criteria
packages/modules/msal/vitest.config.ts Added Vitest configuration for test execution
packages/modules/msal/tsconfig.json Updated TypeScript config to exclude test files from compilation
packages/modules/msal/src/versioning/types.ts TypeScript type definitions for version resolution results
packages/modules/msal/src/versioning/static.ts Enum definitions for version message types
packages/modules/msal/src/versioning/resolve-version.ts Core version resolution logic with permissive checking
packages/modules/msal/src/versioning/index.ts Public API exports for the versioning module
packages/modules/msal/src/versioning/create-version-message.ts Helper function for generating human-readable version messages
packages/modules/msal/src/versioning/VersionError.ts Custom error class for version-related issues
packages/modules/msal/src/v2/provider.ts Updated import path to use new versioning module
packages/modules/msal/src/resolve-version.ts Deleted legacy version resolution file
packages/modules/msal/src/tests/versioning/resolve-version.test.ts Comprehensive test suite for version resolution
packages/modules/msal/package.json Added test script and updated dependencies
packages/modules/msal/README.md Added documentation for version management features

@github-actions github-actions bot added the 🚧 chore maintaines work, (update deps, workflos ...) label Sep 15, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 15, 2025 •
edited
Loading

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 6.4% 37064 / 578607
🔵 Statements 6.4% 37064 / 578607
🔵 Functions 28.67% 316 / 1102
🔵 Branches 49.32% 698 / 1415
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/modules/msal/src/v2/provider.ts 0% 0% 0% 0% 1-169
packages/modules/msal/src/versioning/VersionError.ts 100% 100% 100% 100%
packages/modules/msal/src/versioning/create-version-message.ts 72.41% 50% 100% 72.41% 49, 53, 55, 57-61
packages/modules/msal/src/versioning/index.ts 0% 0% 0% 0% 1-25
packages/modules/msal/src/versioning/resolve-version.ts 89.09% 93.75% 100% 89.09% 70-75
packages/modules/msal/src/versioning/static.ts 100% 100% 100% 100%
packages/modules/msal/src/versioning/types.ts 100% 100% 100% 100%
Generated in workflow #10246 for commit 59ae6a4 by the Vitest Coverage Report Action

@odinr
feat(msal): improve version checking and error handling
cc4f2c1 
- Store version properties as strings instead of SemVer objects in VersionError
- Add warnings array to ResolvedVersion for minor version mismatches
- Improve type safety by using type imports where appropriate
- Collect warnings instead of immediately logging them for better control
@odinr odinr requested a review from Copilot September 15, 2025 10:59
Copilot
Copilot AI reviewed Sep 15, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

github-actions[bot]
github-actions bot reviewed Sep 15, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

tsc

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 140 in cc4f2c1

expect(versionError.requestedVersion.version).toBe('5.0.0');

🚫 [tsc] <2339> reported by reviewdog 🐶
Property 'version' does not exist on type 'string'.

fusion-framework/packages/modules/msal/src/__tests__/versioning/resolve-version.test.ts

Line 141 in cc4f2c1

expect(versionError.latestVersion.version).toBe('4.0.9');

@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from Copilot AI Sep 15, 2025
@odinr
refactor(msal): improve versioning error handling and testing
78729ae 
- Replace console.warn with structured warnings in resolveVersion return
- Update VersionError to use string versions instead of SemVer objects
- Simplify test setup by removing console.warn mocking
- Fix type import for SemVer in resolve-version.ts
- Improve error type consistency (TypeError -> VersionError)
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from Copilot AI Sep 15, 2025
@equinor equinor deleted a comment from Copilot AI Sep 15, 2025
@equinor equinor deleted a comment from Copilot AI Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
@equinor equinor deleted a comment from github-actions bot Sep 15, 2025
Copilot
Copilot AI reviewed Sep 15, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

@equinor equinor deleted a comment from Copilot AI Sep 15, 2025
@odinr
fix(msal): remove unsafe type assertion in resolveVersion function
59ae6a4 
- Replace unsafe type assertion with explicit undefined check
- Throw VersionError when no matching enum version is found
- Update tests to expect errors for unsupported major versions
- Ensures type safety and proper error handling for invalid versions
@github-actions github-actions bot added the 🐞 bug Something isn't working label Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

Copilot code review Copilot Copilot left review comments

@asbjornhaland asbjornhaland asbjornhaland approved these changes

Assignees
No one assigned
Labels
🐞 bug Something isn't working 🚧 chore maintaines work, (update deps, workflos ...) 📚 documentation Improvements or additions to documentation 🚀 feature New feature or request 🧬 Modules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve MSAL module version checking to be more permissive
2 participants
@odinr @asbjornhaland