Remove abandoned BuildBundlerMinifier and fix NSubstitute 6 warnings - #1450
Open
ardalis wants to merge 11 commits into
Open
Remove abandoned BuildBundlerMinifier and fix NSubstitute 6 warnings#1450ardalis wants to merge 11 commits into
ardalis wants to merge 11 commits into
Conversation
Agent-Logs-Url: https://github.com/DevBetterCom/DevBetterWeb/sessions/ecf5bfb1-07ce-4dbb-8be3-2a57358deb71 Co-authored-by: ardalis <782127+ardalis@users.noreply.github.com>
Agent-Logs-Url: https://github.com/DevBetterCom/DevBetterWeb/sessions/ecf5bfb1-07ce-4dbb-8be3-2a57358deb71 Co-authored-by: ardalis <782127+ardalis@users.noreply.github.com>
NSubstitute 6.0 added nullable annotations: CallInfo.Arg<T>() returns T? and Arg.Is<T> predicates receive T?. Suppress with the null-forgiving operator (matchers never actually receive null; NSubstitute swallows predicate exceptions into non-matches either way) and pass null! for the unused UserManager constructor dependencies. Extract the triplicated UserManager substitute recipe into UserManagerHelpers.CreateSubstitute(), following the existing MemberHelpers/SubscriptionHelpers convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BuildBundlerMinifier is archived and its Clean-phase output deletion races the .NET 10 StaticWebAssets pipeline, making dotnet build --no-incremental fail with "No file exists for the asset wwwroot/css/sb-admin-2.min.css". Rather than freeze the generated .min files as hand-maintained commits (a silent-staleness trap: Development served the sources while Staging/Production served .min files nothing regenerates), serve the source files directly in all environments and delete the orphaned artifacts: - site.min.js: was prod-only; both layouts now load site.js everywhere - sb-admin-2.min.css: member layout now loads sb-admin-2.css - site.min.css and landing-page.min.css: referenced by nothing - BundleArtifacts/ .gitignore entry: bundler residue The size cost is small and a follow-up can add response compression, which saves far more than minification did. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related cleanups after the recent dependency updates:
Remove BuildBundlerMinifier (abandoned) and its generated .min assets. The package is archived upstream, and its Clean-phase output deletion races the .NET 10 StaticWebAssets pipeline:
dotnet build --no-incrementalfailed every time with "No file exists for the asset wwwroot/css/sb-admin-2.min.css". Instead of freezing the generated.minfiles as hand-maintained commits — which review flagged as a silent-staleness trap (Development servedsite.jswhile Staging/Production served asite.min.jsthat nothing would ever regenerate, withasp-append-versionhashing the stale file so caches never bust) — both layouts now serve the source files in all environments, and the four orphaned.minartifacts are deleted (site.min.cssandlanding-page.min.csswere referenced by nothing at all).Fix NSubstitute 6.0 nullable warnings in
DevBetterWeb.Tests.CallInfo.Arg<T>()now returnsT?andArg.Is<T>predicates receiveT?; suppressed with!(NSubstitute swallows predicate exceptions into non-matches, so this is behavior-neutral — verified against the decompiled 6.0 matcher). The triplicated 9-nullUserManagersubstitute recipe is extracted intoUserManagerHelpers.CreateSubstitute()alongside the existingMemberHelpers/SubscriptionHelpers.Review notes
An 8-angle automated review ran before commit (the
/codex:reviewplugin required by CLAUDE.md is not installed in this environment, so the built-in review was used instead). Findings addressed: the prod-only stale-JS trap, the orphanedsb-admin-2.css/.minpair, the unreferenced min files, theBundleArtifacts/.gitignore residue, the UserManager duplication, and inconsistent null-suppression idioms. Findings intentionally not taken:o != nullin aDidNotReceivematcher weakens the assertion" — refuted: NSubstitute wraps predicates in try/catch and treats exceptions as non-matches, so the old NRE path already behaved identically; the idiom was switched too!.anyway for consistency._billableStatusesagainst the production array — pre-existing structure, out of scope here.UseResponseCompression) — worth doing as a follow-up; it would save far more bytes than minification ever did (~190KB of member-page CSS compresses to ~25-30KB), which is why serving unminified sources is an acceptable trade today.Testing
dotnet build --no-incremental: previously always failed; now succeeds with 0 warnings, 0 errorsdotnet test: all 233 tests pass (76 + 154 + 3).minfiles,bundleconfig.json, orBuildBundlerMinifier🤖 Generated with Claude Code