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

Adds a router to the built-in serve command #12310

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

markhuot
Copy link
Contributor

When running ./craft serve PHP utilizes it's built-in web server. The built-in web server automatically serves static files with the appropriate mime-type and headers based on the file extension. So, a request to a .php file will route through PHP and return HTML. A request to a .css file will serve the static file with a CSS content-type header. This works really well for actual files on disk.

Unfortunately, if Craft is serving a virtual file (with an extension) through its own internal routing (via a module or otherwise) it will break. For example, insert the following in to a Module,

Event::on(
    UrlManager::class,
    UrlManager::EVENT_REGISTER_SITE_URL_RULES,
    function (RegisterUrlRulesEvent $event) {
        $event->rules['foo.json'] = ['template' => 'my-great-template'];
    }
);

This should serve the template out of the templates directory but instead you get a PHP error that foo.json does not exist. This is because PHP assumes the .json extension is a static file and not something that PHP would normally serve.

To work around this you can call the built in web server with a "router". Adding the router script allows the application code to make a determination if the request is static or not instead of assuming specific extensions are static.

This PR adds a router.php that checks the document root for a file before serving it. If the file does not exist in the filesystem it will fall back to Craft/PHP for serving the file (which mimics the recommended setup for Craft with popular web servers like Nginx).

Note: when running PHP's built in web-server we can be assured that $_SERVER['DOCUMENT_ROOT'] points where we want it too because the Yii serve console command does this for us.

Lastly, it's important to have a separate router.php instead of using index.php as the router. When you specify a router it is (essentially) prepended to all requests. A request to the homepage would (more or less) look like require 'router.php'; require 'index.php'; If the index was your router you end up with a recursive call that times out or just breaks because it tries to prepend index.php on to itself and you get errors with duplicate constants and the like.

--

Long story short: this makes php -S behave more like "production" in that 404'd files route through Craft/PHP.

@markhuot markhuot requested a review from a team as a code owner November 14, 2022 15:18
Copy link
Contributor

@i-just i-just left a comment

Choose a reason for hiding this comment

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

Looks to be working as expected to me.

Copy link
Contributor

@brianjhanson brianjhanson left a comment

Choose a reason for hiding this comment

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

Seems like a solid addition to me.

brandonkelly added a commit that referenced this pull request Nov 28, 2022
@brandonkelly brandonkelly merged commit 2f57c8c into craftcms:develop Nov 28, 2022
@brandonkelly
Copy link
Member

Merged via 96cd72c. Thanks @markhuot!

@brandonkelly
Copy link
Member

Craft 4.3.4 is out with this 🎉

@markhuot
Copy link
Contributor Author

Woo! Made my day, thanks everyone.

I'm a big fan of `php craft serve` for local development and the newly added `router.php` makes it even more powerful when using custom routes. One more paper cut crossed off my list! https://t.co/isC68I95nI

— Mark (@markhuot) November 29, 2022

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.

4 participants