fix: register core before a module whose package sorts ahead of it - #1
Conversation
Laravel registers discovered packages alphabetically, so saucebase/auth (and announcements, billing, blog) reached ModuleServiceProvider::register() before CoreServiceProvider had bound the module registry, and the app failed to boot. Module providers now register core first; it is a no-op when core is already registered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe module service provider now registers ChangesModule provider boot order
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The boot-order regression is addressed and covered by the new feature test; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|



Problem
Laravel registers discovered packages alphabetically. A module whose package name sorts before
saucebase/core— auth, announcements, billing, blog — reachesModuleServiceProvider::register()beforeCoreServiceProviderhas bound internachi'sModuleRegistry.moduleName()then fails and the app does not boot:Core's own tests never saw it because they always list module providers after core's.
Fix
ModuleServiceProvider::register()registersCoreServiceProviderfirst. Registering an already-registered provider is a no-op, so the normal order is unchanged.Tests
ModuleBootOrderTestlists the fixture module's provider before core's and asserts its config is merged. It fails with the error above when the fix is removed.saucebase/authinstalled against this branch: 167 passed.Merging releases v1.0.1.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests