Skip to content
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

Set up API router when first API routes are generated #728

Merged
merged 1 commit into from
Jan 4, 2025

Conversation

nicodevs
Copy link
Contributor

@nicodevs nicodevs commented Jan 3, 2025

Issue

In a clean installation of Laravel 11, the routes/api.php file is absent and not configured in bootstrap/app.php. When Blueprint generates API routes, it creates the file, but the developer must manually edit it to add the PHP opening tag and the use Illuminate\Support\Facades\Route; line. They must also modify bootstrap/app.php to enable that API route file.

Changes

This PR updates Blueprint to check the following when creating API routes:

  • Ensure the routes/api.php file exists, creating it if necessary, using a basic stub
  • Ensure the API routes file is configured in bootstrap/app.php, adding the configuration if missing

These actions occur before the addition of the generated API routes.

This PR takes inspiration from Laravel's ApiInstallCommand.

Testing

Tests to check this functionality has been added to RouteGeneratorTest.php.

PS. No annoying WIPs this time! 😅

@nicodevs
Copy link
Contributor Author

nicodevs commented Jan 3, 2025

The tests are failing, but they are unrelated to this PR and should pass after this is merged: #729

@jasonmccreary
Copy link
Collaborator

I merged the other if you want to rebase this.

@nicodevs
Copy link
Contributor Author

nicodevs commented Jan 3, 2025

Thanks @jasonmccreary ~ Done!

{
$this->filesystem->expects('get')
->with('bootstrap/app.php')
->andReturn("// api: \nweb: __DIR__.'/../routes/web.php',");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this \n passes on Windows...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the hardcoded return value we mock, so it will work (a PHP_EOL would have been better though). However, that part is not entirely necessary, so I streamlined the check here: #730

@jasonmccreary
Copy link
Collaborator

Good PR. There are a few areas that concern me. I commented on the test. But along the same lines, it'd be nice if this had more of an integration tests where it actually changed an existing bootstrap/app.php file.

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