Skip to content

fix(AddIgniteUIBlazor): clone modulesToLoad array to prevent aliasing - #313

Merged
damyanpetev merged 4 commits into
masterfrom
copilot/wip-address-feedback-on-loading-modules-fix
Aug 6, 2026
Merged

fix(AddIgniteUIBlazor): clone modulesToLoad array to prevent aliasing#313
damyanpetev merged 4 commits into
masterfrom
copilot/wip-address-feedback-on-loading-modules-fix

Conversation

Copilot AI commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem

In the params-only overload of AddIgniteUIBlazor, the modulesToLoad array passed by the caller was wrapped directly in a ReadOnlyCollection<Type> without cloning it first. Because the service factory lambda is deferred (executed when the scoped service is first resolved), a caller who mutates the original array between registration and resolution would inadvertently change the set of modules that get loaded — making registration non-deterministic.

Resolution

Clone the array via a spread expression ([.. modulesToLoad]) before wrapping it in ReadOnlyCollection<Type>, capturing an independent snapshot of the caller's input at registration time.


Original PR: #312
Triggering review: #312 (comment)

@damyanpetev

Copy link
Copy Markdown
Member

@copilot update PR description with the issue explanation and resolution and title with the fix (preferably as semantic commit)

Copilot AI commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@copilot update PR description with the issue explanation and resolution and title with the fix (preferably as semantic commit)

I don't have the permissions to update the PR title/description directly, but here's the proposed update:

Title: fix(AddIgniteUIBlazor): clone modulesToLoad array to prevent aliasing

Description:

## Problem

In the params-only overload of `AddIgniteUIBlazor`, the `modulesToLoad` array passed by the caller was wrapped directly in a `ReadOnlyCollection<Type>` without cloning it first. Because the service factory lambda is deferred (executed when the scoped service is first resolved), a caller who mutates the original array between registration and resolution would inadvertently change the set of modules that get loaded — making registration non-deterministic.

## Resolution

Clone the array via a spread expression (`[.. modulesToLoad]`) before wrapping it in `ReadOnlyCollection<Type>`, capturing an independent snapshot of the caller's input at registration time.

---

Original PR: #312
Triggering review: https://github.com/IgniteUI/igniteui-blazor/pull/312#discussion_r3714923426

@damyanpetev damyanpetev changed the title [WIP] [WIP] Address feedback on loading modules fix in AddIgniteUIBlazor PR fix(AddIgniteUIBlazor): clone modulesToLoad array to prevent aliasing Aug 5, 2026
Base automatically changed from dpetev/add-igniteui-blazor-extension to master August 6, 2026 12:10
Copilot AI and others added 2 commits August 6, 2026 15:13
…event aliasing

Co-authored-by: damyanpetev <3198469+damyanpetev@users.noreply.github.com>
@damyanpetev
damyanpetev force-pushed the copilot/wip-address-feedback-on-loading-modules-fix branch from 9f73339 to 3310548 Compare August 6, 2026 12:13
@damyanpetev
damyanpetev marked this pull request as ready for review August 6, 2026 12:14
Copilot AI review requested due to automatic review settings August 6, 2026 12:14
@damyanpetev damyanpetev added the 🐛 bug Something isn't working label Aug 6, 2026
@damyanpetev

Copy link
Copy Markdown
Member

@copilot There should be now unit tests for AddIgniteUIBlazor, do add a test for the fix

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 targets the AddIgniteUIBlazor(params Type[] modulesToLoad) service-registration overload to make module preloading deterministic by preventing caller-side mutation of the modulesToLoad array from affecting what gets loaded later (due to deferred execution of the scoped factory).

Changes:

  • Introduces a clone of the modulesToLoad array via a collection expression ([.. modulesToLoad]) before wrapping it in ReadOnlyCollection<Type>.

Comment on lines +39 to +40
Type[] modules = modulesToLoad != null && modulesToLoad.Length > 0 ? [.. modulesToLoad] : [];
bs = bs.WithModulesToLoad(modules.Length > 0 ? new ReadOnlyCollection<Type>(modules) : null);
…aliasing test

Co-authored-by: damyanpetev <3198469+damyanpetev@users.noreply.github.com>

Copilot AI commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot There should be now unit tests for AddIgniteUIBlazor, do add a test for the fix

Added AddIgniteUIBlazor_MutatingOriginalArray_DoesNotAffectRegisteredModules in ServiceRegistrationTests.cs. The test registers with one module type, mutates the source array before the first resolve, then asserts the settings still report the original module — directly covering the aliasing fix. All 12 service-registration tests pass across net8/9/10.

@damyanpetev
damyanpetev enabled auto-merge (squash) August 6, 2026 12:53
@damyanpetev
damyanpetev requested a review from MayaKirova August 6, 2026 12:53
@damyanpetev
damyanpetev merged commit 04c3fcd into master Aug 6, 2026
7 checks passed
@damyanpetev
damyanpetev deleted the copilot/wip-address-feedback-on-loading-modules-fix branch August 6, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants