Supports Laravel 12 #1164

duncanmcclean · 2025-02-10T15:54:27Z

Summary

This pull request adds Laravel 12 support to graphql-laravel, which will be released on 24th February.

Depends on:

Type of change:

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Misc. change (internal, infrastructure, maintenance, etc.)

Checklist:

Existing tests have been adapted and/or new tests have been added
Add a CHANGELOG.md entry
Update the README.md
Code style has been fixed via composer fix-style

mfn · 2025-02-10T15:56:16Z

Note: I'm already working on it, but as you saw, laragraph/utils#20 is blocking and so is larastan

duncanmcclean · 2025-02-10T15:58:52Z

Note: I'm already working on it, but as you saw, laragraph/utils#20 is blocking and so is larastan

Ah, I thought you might be. I can close this PR if you've already got a branch in the works?

mfn · 2025-02-12T19:48:22Z

No, it's fine :)

We've to wait for the dependencies anyway, so let's wait 💤

mfn · 2025-02-20T13:40:04Z

FTR

[2.x] Supports Laravel 12 larastan/larastan#2198

The PR was closed larastan/larastan#2198 (comment)

I see your point now actually. But package maintainers can use different PHPStan config and baseline for this. At the end it'll not affect the end users, and I really don't want to juggle 4 different Laravel versions.

I can't disagree here, I'm a maintainer myself 😅

So I guess it will be only larastan/larastan#2195

duncanmcclean · 2025-02-20T19:16:38Z

Yeah, it makes sense!

It looks like we may need to require both Larastan 2 and 3. I don't have any experience with Larastan (or PHPStan), but I assume it's okay to use two versions alongside each other?

I can take a look tomorrow when I'm back in the office.

mfn · 2025-02-20T20:13:45Z

I'm fine only keeping one, it's just a dev dependency after all

This reverts commit 6431c2f.

duncanmcclean · 2025-02-21T12:13:41Z

Apologies for the commit spam! 😄

I've been trying to figure out why orchestra/testbench-core 8.x has been getting installed for the Laravel 12 tests, rather than 10.x (which hasn't been tagged, but should still be usable). 🤔

I don't suppose you have any ideas?

mfn · 2025-02-21T13:46:12Z

I supposed it's related to minimum-stability and prefer-stable and the fact that L12 and testbench 10 are aliases to dev versions.

Although it may be possible to work this around before L12 is stable, I think it will be much easier once all the deps transitioned to stable, I believe then things will "just work".

With making more (not commit-table) manually changes I got L12/testbench-core 10 locally.

Test suite runs but one test:

There was 1 failure:

1) Rebing\GraphQL\Tests\Unit\ValidationOfFieldArguments\ValidationOfFieldArgumentsTest::testRulesTakesEffect
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
                     'profile.fields.name.args.includeMiddleNames' => [...]
                     'profile.fields.height.args.unit' => [...]
                     'profile.args.profileId' => Array (
-                        0 => 'The profile.args.profile id may not be greater than 10.'
+                        0 => 'The profile.args.profile id field must not be greater than 10.'
                     )
                 )
             )

/graphql-laravel/tests/Unit/ValidationOfFieldArguments/ValidationOfFieldArgumentsTest.php:92

I suggest: we wait until official release (should be next week or so?), then it won't be that of a hassle.

duncanmcclean · 2025-02-21T14:32:02Z

I suggest: we wait until official release (should be next week or so?), then it won't be that of a hassle.

No problem! I think Laravel 12 is being released on Monday, so I'll loop back to this after it's been tagged.

duncanmcclean · 2025-02-24T15:24:05Z

I've been trying to figure out why orchestra/testbench-core 8.x has been getting installed for the Laravel 12 tests, rather than 10.x (which hasn't been tagged, but should still be usable). 🤔

Unfortunatley, I'm still running into this issue now that everything has been tagged... the prefer-lowest jobs are using Testbench 8 even though they should be using Testbench 10.

Is there any reason you're requiring orchestra/testbench-core instead of orchestra/testbench (which is what most packages seem to require)?

The testbench package seems to include version constraints for the versions of Laravel its intended for, while the testbench-core package doesn't.

Swapping them seemed to have fixed the issues I was seeing with the prefer-lowest tests.

Testbench 8.0 is the minimum version at the moment, so this check is unncessary. (It also seems to be broken - returning true for Testbench 10)

Laravel 12 automatically registers routes for any local filesystems, essentially doing away with the `storage:link` command. These tests needed to be updated to account for that. See https://github.com/laravel/framework/blob/12.x/src/Illuminate/Filesystem/FilesystemServiceProvider.php#L80-L107

duncanmcclean · 2025-02-24T17:42:19Z

This pull request is ready to be reviewed now!

I've opened #1168 to address the PHPStan analysis issues, and #1167 to prevent deprecatication warnings from PHPUnit when using Testbench 10 (which requires PHPUnit 11).

As per my previous comment, I had to switch back to requiring testbench (as apposed to testbench-core) to resolve some dependency issues.

Let me know if there's anything you need me to change. Thanks for maintaining this package! 🙂

mfn · 2025-02-24T19:11:58Z

Is there any reason you're requiring orchestra/testbench-core instead of orchestra/testbench (which is what most packages seem to require)?

f753174

If you ask me now, I guess the answer is "premature optimization" 🤦🏼

mfn · 2025-02-24T19:18:40Z

tests/Unit/NoRoutesRegisteredTest.php

@@ -14,6 +14,10 @@ class NoRoutesRegisteredTest extends TestCase
 {
    protected function getEnvironmentSetUp($app): void
    {
+        // Laravel registers routes for local filesystems by default.
+        // However, for the purpose of this test, we don't want it to register any routes.
+        $app['config']->set('filesystems.disks.local.serve', false);


When is this required? The tests work also without this line for me?

The ->assertCount() here was asserting that 0 was 1, because of the filesystem route that Laravel 12 is registering.

https://github.com/rebing/graphql-laravel/blob/master/tests/Unit/NoRoutesRegisteredTest.php#L54

Looks like it only happens on the prefer-lowest tests. Testbench 10.0.1 includes the serve parameter in its stubs.

mfn

Nice, thanks for your work!

duncanmcclean · 2025-02-24T19:40:31Z

And thanks for getting this tagged so quickly! 🙂

mfn · 2025-02-24T20:05:50Z

Thanks you too, such a great contribution, separate PR for separate concerns, really helped me to keep the velocity, thanks 👍🏼

duncanmcclean added 3 commits February 10, 2025 12:37

Add Laravel 12 version constraints

913f569

Update matrix in GitHub Actions workflow

e3e3c9d

Add changelog entry

dd7f579

duncanmcclean mentioned this pull request Feb 10, 2025

[5.x] Laravel 12 statamic/cms#11433

Merged

4 tasks

mfn mentioned this pull request Feb 19, 2025

Laravel 12.x Compatibility #1166

Closed

duncanmcclean added 9 commits February 21, 2025 10:18

Require Larastan ^3

7ec6295

Laravel 9 is no longer supported. This can be removed.

6431c2f

Require Testbench like this for the minute.

d557dd1

wip

150eec2

wip

bad4e01

Revert "Laravel 9 is no longer supported. This can be removed."

3034d79

This reverts commit 6431c2f.

Remove only the portion we need to remove.

965f7e2

Go back to how we were requiring Testbench 10 originally.

d729458

wip

3889351

duncanmcclean force-pushed the laravel-12 branch from 1b8f36a to 3889351 Compare February 21, 2025 11:52

duncanmcclean added 2 commits February 21, 2025 11:55

wip

0d8516c

wip

0f1fa9a

Empty commit

a8d412d

duncanmcclean added 2 commits February 24, 2025 16:02

Remove unnecessary version check in tests

a82fc9a

Testbench 8.0 is the minimum version at the moment, so this check is unncessary. (It also seems to be broken - returning true for Testbench 10)

Swap testbench for testbench-core to see if it fixes the tests

64b2afc

duncanmcclean added 3 commits February 24, 2025 16:07

Testbench 10 requires PHPUnit 11

cd2bbd2

Code Style

f256332

duncanmcclean marked this pull request as ready for review February 24, 2025 17:39

duncanmcclean mentioned this pull request Feb 24, 2025

PHPStan 2 #1168

Merged

9 tasks

mfn reviewed Feb 24, 2025

View reviewed changes

mfn approved these changes Feb 24, 2025

View reviewed changes

mfn merged commit 8a9de2d into rebing:master Feb 24, 2025
28 of 29 checks passed

duncanmcclean deleted the laravel-12 branch February 24, 2025 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Supports Laravel 12 #1164

Supports Laravel 12 #1164

duncanmcclean commented Feb 10, 2025 •

edited

Loading

mfn commented Feb 10, 2025

duncanmcclean commented Feb 10, 2025 •

edited

Loading

mfn commented Feb 12, 2025

mfn commented Feb 20, 2025

duncanmcclean commented Feb 20, 2025 •

edited

Loading

mfn commented Feb 20, 2025

duncanmcclean commented Feb 21, 2025

mfn commented Feb 21, 2025

duncanmcclean commented Feb 21, 2025

duncanmcclean commented Feb 24, 2025 •

edited

Loading

duncanmcclean commented Feb 24, 2025 •

edited

Loading

mfn commented Feb 24, 2025

mfn Feb 24, 2025

duncanmcclean Feb 24, 2025

mfn Feb 24, 2025

mfn left a comment

duncanmcclean commented Feb 24, 2025

mfn commented Feb 24, 2025

Supports Laravel 12 #1164

Supports Laravel 12 #1164

Conversation

duncanmcclean commented Feb 10, 2025 • edited Loading

Summary

mfn commented Feb 10, 2025

duncanmcclean commented Feb 10, 2025 • edited Loading

mfn commented Feb 12, 2025

mfn commented Feb 20, 2025

duncanmcclean commented Feb 20, 2025 • edited Loading

mfn commented Feb 20, 2025

duncanmcclean commented Feb 21, 2025

mfn commented Feb 21, 2025

duncanmcclean commented Feb 21, 2025

duncanmcclean commented Feb 24, 2025 • edited Loading

duncanmcclean commented Feb 24, 2025 • edited Loading

mfn commented Feb 24, 2025

mfn Feb 24, 2025

Choose a reason for hiding this comment

duncanmcclean Feb 24, 2025

Choose a reason for hiding this comment

mfn Feb 24, 2025

Choose a reason for hiding this comment

mfn left a comment

Choose a reason for hiding this comment

duncanmcclean commented Feb 24, 2025

mfn commented Feb 24, 2025

duncanmcclean commented Feb 10, 2025 •

edited

Loading

duncanmcclean commented Feb 10, 2025 •

edited

Loading

duncanmcclean commented Feb 20, 2025 •

edited

Loading

duncanmcclean commented Feb 24, 2025 •

edited

Loading

duncanmcclean commented Feb 24, 2025 •

edited

Loading