-
-
Notifications
You must be signed in to change notification settings - Fork 15
feat: framework refactor + decouple from Hyperf #349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
binaryfire
wants to merge
560
commits into
hypervel:0.4
Choose a base branch
from
binaryfire:feature/hyperf-decouple
base: 0.4
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+127,890
−9,446
Conversation
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
Add native types to where* filter methods. Remove redundant @param docblocks where native types suffice.
Restore generic type information that was incorrectly removed from intersect* methods. Generic types like <TKey, TValue> cannot be expressed in native PHP and must be preserved in docblocks for static analysis.
- Added native PHP 8.2+ types to all remaining interface methods - Preserved generic @param/@return docblocks where they contain type info (TKey, TValue, etc.) - Removed redundant docblocks where native types fully express the same info - Shortened namespace references (Arrayable instead of \Illuminate\Contracts\Support\Arrayable)
- Added native types to all methods missing them - Fixed whereIn/whereInStrict/whereBetween/whereNotBetween/whereNotIn/whereNotInStrict to use Arrayable|iterable instead of mixed - Removed redundant @param/@return docblocks where native types express same info - Preserved generic @param/@return docblocks (TKey, TValue, etc.) - Shortened namespace references in docblocks - Removed redundant @return $this (replaced by native : static)
- Changed makeIterator() return type from Traversable to Iterator - Wrap non-Iterator Traversables with IteratorIterator to ensure Iterator methods (valid, next, current, key) are available - Changed getIterator() return type from Traversable to Iterator (PHP allows covariant return types) - Fixed chunk() closure parameter types from Traversable to Iterator This fixes 55 PHPStan method.notFound errors where Iterator methods were being called on Traversable types.
Changed callable(...mixed): mixed to callable(mixed...): mixed which is the correct PHPStan syntax for variadic parameters.
Wrapped callable return types in parentheses to prevent PHPStan from parsing int as a separate union member: - (callable(TValue): float|int) -> (callable(TValue): (float|int)) Fixed in Enumerable.php and EnumeratesValues.php
Changed from mixed $items to Arrayable|iterable ...$items to match the Enumerable interface signature. Also changed func_get_args() to $items since we now have the variadic parameter directly.
Changed return type from mixed to string to match Enumerable interface. Cast single item to string for consistency with how implode() handles multiple items - both convert values to strings.
Add Arrayable to the parameter type union for LSP contravariance compliance. The interface specifies Arrayable|iterable, so the implementation must accept at least those types. Handle Arrayable by converting to array before passing to makeIterator().
Use static<int, TValue> instead of Collection<TKey, TValue> for the callback's chunk parameter to match the Enumerable interface's callable signature and satisfy PHPStan contravariance checks.
floor() returns float even for whole numbers. Cast to int where functions expect integer parameters: times() and array_slice().
- collapseWithKeys(): static<mixed, mixed> -> static<array-key, mixed> - crossJoin(): constrain TCrossJoinKey template to array-key Keys in PHP arrays must be int|string (array-key). Using mixed as a key type violates the class template constraint.
- pluck(): Remove dead instanceof Closure checks - $value is string|array (never Closure), $key is ?string (never Closure) - only()/select(): Remove dead is_null() checks - native type Enumerable|array|string doesn't accept null - except(): Add null to PHPDoc since native type is mixed and code handles null case
After checking for IteratorAggregate and array, only callable remains from the union type - the is_callable() check and final fallback return were unreachable dead code.
guessResourceName() returns array<int, class-string<...>> - elements are always strings. String concatenation also always produces strings. The is_string() checks before class_exists() were dead code.
- LazyCollection::first() callable should accept (TValue, TKey) to match interface and actual usage - resolveResourceFromAttribute/resolveResourceCollectionFromAttribute accept any class-string (typically Model), not just resource classes. Fixed param types and improved return type specificity.
Each ignore is documented with explanation: Type system limitations (can't express constraint): - flip()/combine(): TValue becomes key, only valid when TValue is array-key - groupBy()/keyBy(): Complex conditional return types PHPStan can't match Type narrowing PHPStan can't track: - ensure(): Throws if items don't match type - whereInstanceOf(): Filter only keeps matching instances - partition(): Returns exactly 2 elements with keys 0,1 Passthru pattern loses generic info: - crossJoin, mapToDictionary, merge, mergeRecursive, split Defensive code (runtime validation): - method_exists for trait presence - instanceof checks for non-Generator callables - Validation checks for negative/zero input values - CachingIterator accepts any int flags Callback type mismatches: - Collection passes callback to LazyCollection with different chunk type - chunkWhile callback typed for static but receives Collection
- Replace all use Hyperf\Collection\* imports with Hypervel\Support\* - Remove use function Hyperf\Collection\* imports (global functions available) - Update FQCN references in PHPDoc and return types - Replace hyperf/collection dependency with hypervel/collections in 14 composer.json files - Port support/helpers.php from Laravel, keeping Swoole-specific environment() function
- Remove duplicate enum_value() from support/Functions.php (keep collections version) - Update contract imports: Arrayable, Jsonable, CanBeEscapedWhenCastToString -> Hypervel\Contracts\Support - Fix EnumeratesValues to use Hypervel\Support\Traits\Conditionable - Fix Conditionable trait signatures to match Enumerable interface - Fix LazyCollection::get() to return null instead of bare return
- Change return types from Enumerable to static - Fix parameter types to match parent signatures - Methods: map, mapWithKeys, collapse, flatten, flip, keys, pad, pluck, zip, countBy, duplicateComparator
The second parameter can be a value (not operator) when called with 2 args
PHP's type system requires covariant return types, but Eloquent\Collection needs to return base Collection from methods like map() when items are no longer Model instances. Laravel omits return types on these methods for this flexibility. Methods updated: - map, mapWithKeys, collapse, flatten, flip, keys, pad, pluck, zip, countBy, partition Updated in: - Enumerable interface - Collection - LazyCollection - Eloquent\Collection
- Change increment() and decrement() from protected to public to match Laravel (needed for Builder::incrementOrCreate) - Remove unreachable instanceof QueueableEntity check in Collection (all Models implement QueueableEntity, so the fallback was dead code)
Type fixes after porting Laravel packages with strict types: - HasTimestamps: Use CarbonInterface return type for freshTimestamp() (allows Carbon or CarbonImmutable depending on Date facade config) - BelongsToRelationship: Use int|string|null for $resolved property (caches primary key which can be int or string) - BelongsTo: Use ?string for getOwnerKeyName() return type (ownerKey property is nullable, especially for MorphTo) - InteractsWithPivotTable: Use int|string for formatAttachRecord $key (array keys can be numeric indices or string IDs like UUIDs) - Sequence: Use array|Model for __invoke $attributes parameter (receives array in factory state context, Model in pivot context) - TestCase: Bind Faker\Generator in container for factory tests (Laravel expects Generator bound, uses Faker\Factory::create) - DateFactoryTest: Use $casts instead of deprecated $dates property (Laravel deprecated $dates in favor of $casts)
chore: add types analysis to CI workflow
The ported Laravel Grammar class now requires a Connection argument.
- Add static $resolvedBuilderClasses property to Model for caching - Update newEloquentBuilder to use cache (Hypervel optimization) - Rename newModelBuilder to newEloquentBuilder in test (Laravel naming)
…eritance) Laravel does NOT inherit ScopedBy attributes from parent classes. PHP attributes are not inherited by default, and Laravel does not implement custom inheritance logic. Updated tests to verify this behavior is preserved (the old Hypervel added inheritance which would be a Laravel API deviation).
Ported: - DatabaseProcessorTest (1 test) - DatabaseQueryExceptionTest (9 tests) - renamed getConnection to avoid conflict - DatabaseQueryGrammarTest (4 tests) - DatabaseSchemaBlueprintTest (37 tests) - removed SqlServer assertions Fixed fixture models with typed properties: - EloquentModelUsingNonIncrementedInt - EloquentModelUsingUlid - EloquentModelUsingUuid - User (extends Model + uses Authenticatable trait)
- Use Testbench\TestCase instead of manual container setup - Add SQLiteGrammar with connection parameter (Hypervel's Grammar requires Connection)
Replace Mockery spy on callable with closure capturing args for strict type compatibility.
Add RunTestsInCoroutine trait for Context isolation between tests. Update porting guide with coroutine testing patterns.
Model and Query\Builder increment/decrement methods now accept mixed $amount to allow custom casters to pass their own types (e.g., Euro objects). Query\Builder validates with is_numeric() inside the method to match Laravel's behavior. Also port EloquentModelCustomCastingTest with strict typing fixes.
- Use Testbench for container access to event dispatcher - Add connection name to avoid null assignment error
- Update namespaces from Illuminate to Hypervel - Remove SqlServer-specific tests (SqlServer not supported) - Additional SQLite tests ported
SqlServer is not supported in Hypervel.
- Grammar::wrap(): return string|int|float to match getValue() - Builder::$from: make nullable with default null - Builder::limit()/offset(): accept null to reset values - Builder::orWhereNotNull(): accept array like whereNotNull() - Builder::whereBetweenColumns(): add subquery support like whereBetween() - Builder::whereDate/Time/Day/Month/Year and or* variants: use mixed for operator/value params to accept int, Expression, etc.
Source code fixes: - fromRaw(): accept Expression|string, assign directly if Expression - join()/joinSub(): accept mixed for $second param (used as value when $where=true) - orderByRaw(): accept mixed bindings (addBinding already handles scalars) - insertGetId(): return int|string to match processor signatures - forNestedWhere(): handle null $from gracefully - selectExpression(): add missing method from Laravel - cloneWithout(): reset arrays to [] instead of null - find(): return object|array|null to match actual behavior - incrementEach(): add missing validation for non-numeric/non-associative inputs Test fixes: - Change expected exception from InvalidArgumentException to TypeError for tests that pass invalid types to strictly-typed methods - Fix cursor paginate mock to return Collection instead of array
- DatabaseSchemaBuilderIntegrationTest: use Hypervel Capsule/Manager - DatabaseSchemaBuilderTest: update mocks to use Grammar class - DatabaseSeederTest: use Hypervel Console/Command and Container - DatabaseSoftDeletingScopeTest: update all imports - DatabaseSoftDeletingTest: use Testbench for Date facade, add typed properties - DatabaseSoftDeletingTraitTest: fix save() return type
Renamed classes that were defined in multiple test files with the same name in the same namespace: - EloquentTestUser → TablePrefixEloquentTestUser (in TablePrefixTest) - HasOneInverseChildModel → HasOneRelationInverseChildModel (in HasOneTest) - HasOneInverseChildModel → InverseRelationChildModel (in InverseRelationTest) - TestModel → CircularRecursionTestModel (in CircularRecursionTest) - TestModel → StrictMorphsTestModel (in StrictMorphsTest) The primary IntegrationTest keeps EloquentTestUser as-is since it has 248 usages vs 4 in TablePrefixTest.
- Add ComparesCastableAttributes contract for custom equality checking - Add DeviatesCastableAttributes contract for increment/decrement operations - Add SerializesCastableAttributes contract for array serialization - Remove DatabaseMigratorIntegrationTest (requires Illuminate dependencies) - Update composer.json audit config
Renamed conflicting class names: - User/Article → BelongsToManyWithoutTouchingUser/Article (in WithoutTouchingTest) - User/Post → EloquentIntegrationUser/Post (in IntegrationTest) - Post/CustomPost → RelationshipsPost/CustomPost (in RelationshipsTest) Fixed EloquentModelCustomCastingTest tearDown to drop 'people' table. Remaining 114 test errors are similar missing table cleanup in other test files - each creates tables in setUp but doesn't drop all of them in tearDown.
Previously, RegisterSQLiteConnectionListener registered a global resolver via Connection::resolverFor() that persisted in-memory SQLite PDOs in ApplicationContext. This caused test isolation issues because: 1. Connection::$resolvers is static (process-global) 2. The listener ran on BootApplication, affecting ALL SQLite connections 3. Capsule tests would share the same PDO, causing "table already exists" The new architecture moves persistence into DbPool where it belongs: - DbPool detects in-memory SQLite and creates a shared PDO via factory - All pool slots share this PDO (correct for pooling semantics) - Capsule/non-pooled paths never see this logic (proper isolation) - ConnectionFactory gains makeFromPdo() to create connections from existing PDO This is architecturally cleaner because: - Pooling concerns stay in pooling code - No global static state pollution - Factory remains the single source of Connection creation - Clear ownership: pool owns shared PDO lifecycle
Renames: - makeFromPdo() → makeSqliteFromSharedPdo() (explicit purpose) - $sharedInMemoryPdo → $sharedInMemorySqlitePdo (clarifies in-memory only) - getSharedPdo() → getSharedInMemorySqlitePdo() - createSharedPdo() → createSharedInMemorySqlitePdo() Improvements: - makeSqliteFromSharedPdo() now uses getWriteConfig() for consistency - makeSqliteFromSharedPdo() calls createConnection() to respect custom resolvers - refresh() now rebinds to shared PDO instead of creating fresh empty database Tests added (23 total): - DbPool::isInMemorySqlite() detection (10 cases via data provider) - Shared PDO creation and lifecycle - All pool slots share same PDO for in-memory SQLite - Data persists across pool slots - Capsule connections are isolated from pooled connections - Multiple Capsule instances are isolated from each other - Each Capsule gets its own fresh PDO Tests removed: - RegisterSQLiteConnectionListenerTest.php (tested deleted listener)
The setUpFaker() closure was capturing $this->app which becomes null after tearDown(). When a non-Foundation test later tried to use Model::factory(), the stale binding would fail with "Call to member function make() on null". Changed to use the standard Laravel pattern of receiving $app as the closure parameter, which Hyperf's FactoryResolver already passes.
The test was setting RequestContext with a mock but not clearing it in tearDown. This caused Sanctum tests to fail when run after Foundation tests because SanctumGuard picked up the stale mock.
Tests using RunTestsInCoroutine were polluting Context state that persisted between tests. Adding Context::destroyAll() in the finally block ensures each test starts with a clean slate. Fixes 8 DatabaseTransactionsManagerTest failures caused by transaction state leaking between tests.
The new Context-based Model guarding (replacing static $unguarded) properly isolates guard state per coroutine. Combined with Context::destroyAll() in test teardown, tests can no longer rely on leaked unguarded state from previous tests. Fixes: - NestedSet QueryBuilder: exclude primary key from fill() in rebuildTree since the model already has its ID set - NestedSet NodeTest: use 'name' instead of 'title' (matches $fillable) - Permission tests: remove is_forbidden from Permission::create() since it's a pivot column, not a column on the permissions table
Context::destroyAll() was destroying data needed by defer callbacks (e.g., Redis connections waiting to be released). Replace with targeted cleanup that only destroys specific pollution-prone keys: - Transaction manager state (__db.transactions.*) - Model guard state (__database.model.unguarded) This fixes the Mockery warnings about release() not being called while still preventing test pollution from leaked transaction/guard state.
- Add inline ignore for is_string runtime validation in Query/Builder - Add pattern for Conditionable::when() callback that only needs 1 param - Add path-scoped ignores for Conditionable trait PHPStan limitations (return.type, arguments.count, nullCoalesce.expr)
find() has explicit return type object|array|null in its method signature, unlike first() which uses the generic TValue (stdClass) from BuildsQueries.
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.
Hi @albertcht. This isn't ready yet but I'm opening it as a draft so we can begin discussions and code reviews. The goal of this PR is to refactor Hypervel to be a fully standalone framework that is as close to 1:1 parity with Laravel as possible.
Why one large PR
Sorry about the size of this PR. I tried spreading things across multiple branches but it made my work a lot more difficult. This is effectively a framework refactor - the database package is tightly coupled to many other packages (collections, pagination, pool) as well as several support classes, so all these things need to be updated together. Splitting it across branches would mean each branch needs multiple temporary workarounds + would have failing tests until merged together, making review and CI impractical.
A single large, reviewable PR is less risky than a stack of dependent branches that can't pass CI independently.
Reasons for the refactor
1. Outdated Hyperf packages
It's been difficult to migrate existing Laravel projects to Hypervel because Hyperf's database packages are quite outdated. There are almost 100 missing methods, missing traits, it doesn't support nested transactions, there are old Laravel bugs which haven't been fixed (eg. JSON indices aren't handled correctly), coroutine safety issues (eg. model
unguard(),withoutTouching()). Other packages like pagination, collections and support are outdated too.Stringablewas missing a bunch of methods and traits, for example. There are just too many to PR to Hyperf at this point.2. Faster framework development
We need to be able to move quickly and waiting for Hyperf maintainers to merge things adds a lot of friction to framework development. Decoupling means we don't need to work around things like PHP 8.4 compatibility while waiting for it to be added upstream. Hyperf's testing package uses PHPUnit 10 so we can't update to PHPUnit 13 (and Pest 4 in the skeleton) when it releases in a couple of weeks. v13 has the fix that allows
RunTestsInCoroutineto work with newer PHPUnit versions. There are lots of examples like this.3. Parity with Laravel
We need to avoid the same drift from Laravel that's happened with Hyperf since 2019. If we're not proactive with regularly merging Laravel updates every week we'll end up in the same situation. Having a 1:1 directory and code structure to Laravel whenever possible will make this much easier. Especially when using AI tools.
Most importantly, we need to make it easier for Laravel developers to use and contribute to the framework. That means following the same APIs and directory structures and only modifying code when there's a good reason to (coroutine safety, performance, type modernisation etc).
Right now the Hypervel codebase is confusing for both Laravel developers and AI tools:
hypervel/contractspackage, the Hyperf database code is split across 3 packages, the Hyperf pagination package ishyperf/paginatorand nothyperf/pagination)static::registerCallback('creating')vsstatic::creating())ConfigProviderand LaravelServiceProviderpatterns across different packages is confusing for anyone who doesn't know HyperfThis makes it difficult for Laravel developers to port over apps and to contribute to the framework.
4. AI
The above issues mean that AI needs a lot of guidance to understand the Hypervel codebase and generate Hypervel boilerplate. A few examples:
hypervel/contractsfor contracts) and then have to spend a lot of time grepping for things to find them.And so on... This greatly limits the effectiveness of building Hypervel apps with AI. Unfortunately MCP docs servers and CLAUDE.md rules don't solve all these problems - LLMs aren't great at following instructions well and the sheer volume of Laravel data they've trained on means they always default to Laravel-style code. The only solution is 1:1 parity. Small improvements such as adding native type hints are fine - models can solve that kind of thing quickly from exception messages.
What changed so far
New packages
illuminate/databaseportilluminate/collectionsportilluminate/paginationportilluminate/contracts)hyperf/pool)Macroableto a separate package for Laravel parityRemoved Hyperf dependencies so far
Database package
The big task was porting the database package, making it coroutine safe, implementing performance improvements like static caching and modernising the types.
whereLike,whereNot,groupLimit,rawValue,soleValue, JSON operations, etc.Collections package
Contracts package
Support package
hyperf/tappable,hyperf/stringable,hyperf/macroable,hyperf/codecdependenciesStr,Envand helper classes from LaravelHypervel\Contextwrappers (will be portinghyperf/contextsoon)Number::useCurrency()wasn't actually setting the currency)Coroutine safety
withoutEvents(),withoutBroadcasting(),withoutTouching()now use Context instead of static propertiesUnsetContextInTaskWorkerListenerto clear database context in task workersConnection::resetForPool()to prevent state leaks between coroutinesDatabaseTransactionsManagercoroutine-safeBenefits
Testing status so far
What's left (WIP)
The refactor process
Hyperf's Swoole packages like
pool,coroutine,contextandhttp-serverhaven't changed in many years so porting these is straightforward. A lot of the code can be simplified since we don't need SWOW support. And we can still support the ecosystem by contributing any improvements we make back to Hyperf in separate PRs.Eventually I'll refactor the bigger pieces like the container (contextual binding would be nice!) and the config system (completely drop
ConfigProviderand move entirely to service providers). But those will be future PRs. For now the main refactors are the database layer, collections and support classes + the simple Hyperf packages. I'll just port the container and config packages as-is for now.Let me know if you have any feedback, questions or suggestions. I'm happy to make any changes you want. I suggest we just work through this gradually, as an ongoing task over the next month or so. I'll continue working in this branch and ping you each time I add something new.