Skip to content

fix: register core before a module whose package sorts ahead of it - #1

Merged
sauce-base merged 1 commit into
mainfrom
dev-v3
Sep 11, 2026
Merged

sauce-base merged 1 commit into
mainfrom
dev-v3

Conversation

@roble

@roble roble commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Laravel registers discovered packages alphabetically. A module whose package name sorts before saucebase/core — auth, announcements, billing, blog — reaches ModuleServiceProvider::register() before CoreServiceProvider has bound internachi's ModuleRegistry. moduleName() then fails and the app does not boot:

Unresolvable dependency resolving [Parameter #0 [ <required> string $modules_path ]] in class InterNACHI\Modular\Support\ModuleRegistry

Core's own tests never saw it because they always list module providers after core's.

Fix

ModuleServiceProvider::register() registers CoreServiceProvider first. Registering an already-registered provider is a no-op, so the normal order is unchanged.

Tests

  • ModuleBootOrderTest lists 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.
  • Core suite: 138 passed. PHPStan: no errors.
  • Verified in the app with saucebase/auth installed against this branch: 167 passed.

Merging releases v1.0.1.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed module startup ordering so modules discovered before the core module can still load and merge their configuration correctly.
  • Tests

    • Added coverage to verify configuration remains available when a module is registered before the core provider.

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>
@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9cab778f-7a6e-42d6-a620-1d78809f39e7

📥 Commits

Reviewing files that changed from the base of the PR and between c8e7188 and fcb8375.

📒 Files selected for processing (2)
  • src/Providers/ModuleServiceProvider.php
  • tests/Feature/ModuleBootOrderTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The module service provider now registers CoreServiceProvider before configuration merging and module provider registration. A feature test verifies that a module discovered before the core provider still merges its configuration.

Changes

Module provider boot order

Layer / File(s) Summary
Core provider registration and boot-order test
src/Providers/ModuleServiceProvider.php, tests/Feature/ModuleBootOrderTest.php
register() imports and registers CoreServiceProvider before configuration merging and the module provider loop. The feature test prepends fixture module providers and verifies the expected module configuration value.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to fcb83

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: registering core before modules that sort ahead of it to fix boot ordering.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-v3

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@sauce-base
sauce-base merged commit 6432edc into main Sep 11, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants