fix: admin hub tiles + Pages/Index.tsx barrel collision detection#139
Merged
antosubash merged 3 commits intomainfrom Apr 30, 2026
Merged
fix: admin hub tiles + Pages/Index.tsx barrel collision detection#139antosubash merged 3 commits intomainfrom
antosubash merged 3 commits intomainfrom
Conversation
…lision - Admin hub now probes the registered EndpointDataSource and only renders tiles whose target route is actually registered. Minimal installs no longer surface 404 tiles for missing peer modules (#134). - defineModuleConfig validates Pages/index.ts at config-evaluation time and throws a clear, actionable error when the barrel imports a case-insensitively colliding sibling (e.g. import('./Index') next to Pages/Index.tsx). Prevents the silent self-referential chunk that breaks builds on macOS/Windows (#131).
Deploying simplemodule-website with
|
| Latest commit: |
06be810
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://689a1f10.simplemodule-website.pages.dev |
| Branch Preview URL: | https://claude-fix-issues-lbpo6.simplemodule-website.pages.dev |
- Cache the computed availableUrls in HubEndpoint instead of rebuilding the registered-route set on every request. The endpoint table is fixed after startup so the first request populates the cache. - Drop the redundant existsSync pre-flight in assertNoBarrelCollision and rely on the existing readdirSync try/catch. Tighten the import-detection regex to look for `./index` directly rather than capture-then-compare.
10 tasks
antosubash
added a commit
that referenced
this pull request
Apr 30, 2026
* fix: address remaining issues from downstream feedback batch Closes the four issues left open after #127 and #139: - #137 — `sm new module` now scaffolds a `<Name>Permissions.cs` so the IModulePermissions pattern is discoverable without source-diving. SimpleModule.Permissions README documents the convention with a worked example, and the constitution clarifies that ConfigurePermissions is no longer required (auto-discovered). - #135 — adds `scripts/smoke-test-nupkg-consumer.mjs` plus a CI step that builds a throwaway PackageReference-based consumer against the freshly packed nupkgs and asserts each module's static web assets propagate through the static-asset manifest. Catches the regression class where the package looks fine on disk but the consumer's wwwroot pipeline doesn't pick the assets up — invisible to the in-repo Host because it uses ProjectReference. - #133 — extracts `framework/SimpleModule.Testing` (TestAuthHandler, TestAuthDefaults, AddTestAuthentication, CreateAuthenticatedClient WebApplicationFactory extensions) so downstream apps can pull the helpers from NuGet instead of copy-pasting. Tests.Shared now consumes it; existing 40+28+20 (Users/Admin/OpenIddict) and 209/195 (Core/Generator) tests stay green. - #130 — adds `ModuleGraphValidator` (opt-in via `SimpleModuleOptions.ValidateModuleGraph`, default on outside Production). Walks loaded SimpleModule.X.Contracts assemblies and logs a warning for every contract interface with no registered implementation — the typical signal that the impl module's package is missing. * refactor: simplify based on /simplify findings - ModuleGraphValidator now uses IServiceProviderIsService.IsService instead of GetService, avoiding eager construction of every contract impl + a redundant scope at startup. - WebApplicationFactoryAuthExtensions uses WellKnownClaims.Permission instead of the bare "permission" string. - Smoke-test consumer puts its NuGet global-packages folder outside the project tree so the SDK's default **/*.cs glob can't reach into it, removing the DefaultItemExcludes workaround. - Drops the now-unused TestAuthScheme const alias on SimpleModuleWebApplicationFactory; remaining call site uses TestAuthDefaults.AuthenticationScheme directly. * ci: allowlist SimpleModule.Testing in framework scope check
This was referenced Apr 30, 2026
Closed
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
2 participants
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.
Addresses two of the open issues raised in the recent downstream-feedback batch.
#134 — Admin hub now hides tiles for unregistered modules
/adminrendered tiles for Tenants, Pages, Email Templates / History, Menus, Feature Flags, Rate Limiting, Background Jobs, and Audit Logs unconditionally. In a minimal install only Users / Roles / OAuth Clients / App Settings actually resolve — the other tiles 404'd, presenting the hub as if every peer module were installed.HubEndpointnow injectsEndpointDataSource, snapshots the set of registered route patterns, and passes only the URLs that resolve through to the React page.Hub.tsxfilters card items against that set and drops empty groups.Lightweight option-1 fix from the issue. The cleaner option-2 (each module contributes its own tile via a discoverable convention) is still on the table for follow-up — this PR keeps the change surface tiny while removing the broken UX.
#131 —
defineModuleConfigrejects thePages/Index.tsxbarrel-collision trapA module with
Pages/Index.tsxplus the framework-requiredPages/index.tsbarrel doing() => import('./Index')builds silently broken on macOS/Windows. Rolldown resolves./Indexto the case-insensitive sibling./index.ts(the barrel itself), emits a self-referential chunk, and the page crashes at runtime withCannot assign to property 'layout' of [object Module].defineModuleConfignow scans the barrel at config-evaluation time. If it contains an extension-less dynamic import whose specifier case-insensitively equalsindexand a colliding sibling page (e.g.Index.tsx) exists, it throws with an actionable message:Verified the validator catches the trap and does not false-positive on the normal sibling case (
Pages/Browse.tsx+import('./Browse')).Other open issues (not in this PR)
PermissionRegistryBuilder.AddPermissions) — pure docs/template work; deferred.SimpleModule.Tests.Sharedto NuGet) — currently project-referencesSimpleModule.Hostand ~20 module projects; non-trivial extraction needed before it can be safely packed. Deferred.Test plan
dotnet buildof the Admin module succeeds.dotnet test modules/Admin/tests/SimpleModule.Admin.Tests— 28 passed, 1 skipped.Pages/Index.tsx+import('./Index')(manual repro).Pages/Browse.tsx+import('./Browse').biome checkandcsharpier checkclean on changed files./adminin a minimal-install consumer to confirm only registered tiles render.Generated by Claude Code