Skip to content

Update Route.php #7

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

Merged
merged 1 commit into from
Jan 25, 2021
Merged

Update Route.php #7

merged 1 commit into from
Jan 25, 2021

Conversation

joanhey
Copy link
Contributor

@joanhey joanhey commented Jan 11, 2021

Fix:
TypeError(0):str_replace(): Argument #3 ($subject) must be of type array|string, null given
/duckphp/vendor/dvaknheo/duckphp/src/Core/Route.php : 274

Fix:
TypeError(0):str_replace(): Argument dvaknheo#3 ($subject) must be of type array|string, null given
/duckphp/vendor/dvaknheo/duckphp/src/Core/Route.php : 274
@dvaknheo
Copy link
Owner

In v1.2.10 stable version , I change resolve by judge type,
$base_class = str_replace('~', $this->namespace_prefix, $this->options['controller_base_class'] ?? '');
at now ,when i found this pr , ok , you are the better way .
I may thinked developer can take not only string , to is_subclass_of() at i first write that code

@dvaknheo dvaknheo merged commit 4d3f458 into dvaknheo:master Jan 25, 2021
@joanhey
Copy link
Contributor Author

joanhey commented Jan 25, 2021

I found the problem in the techempower benchmark.

I updated the bench with v1.2.10 and PHP8:
TechEmpower/FrameworkBenchmarks#6311

And changed to a faster array_combine in the fortunes test.
TechEmpower/FrameworkBenchmarks@ef86539

Now it's faster, later I will add the JIT.

@joanhey joanhey deleted the patch-2 branch January 25, 2021 11:45
@dvaknheo
Copy link
Owner

I found the problem in the techempower benchmark.

I updated the bench with v1.2.10 and PHP8:
TechEmpower/FrameworkBenchmarks#6311

And changed to a faster array_combine in the fortunes test.
TechEmpower/FrameworkBenchmarks@ef86539

Now it's faster, later I will add the JIT.

thank you very much . It's grad , I am planning to review the code today and tomorrow.

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