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

add route group #241

Closed
wants to merge 3 commits into from
Closed

add route group #241

wants to merge 3 commits into from

Conversation

wpjscc
Copy link

@wpjscc wpjscc commented Oct 18, 2023

   $app->addGroup('/users', [
        // middleware1,
        // middleware2
    ], function ($app) {
        $app->get('/{name}', function ($request) {
            return \React\Http\Message\Response::plaintext(
                "Hello " . $request->getAttribute('name') . "!\n"
            );
        });
        $app->addGroup('/{name}/posts', [
            // middleware1,
            // middleware2
        ], function ($app) {
            $app->get('/{post}', function ($request) {
                return \React\Http\Message\Response::plaintext(
                    "Hello User {$request->getAttribute('name')} Posts " . $request->getAttribute('post') . "!\n"
                );
            });
        });
    });

    $app->addGroup('/posts', [
        // middleware1,
        // middleware2
    ], function ($app) {
        $app->get('/{post}', function ($request) {
            return \React\Http\Message\Response::plaintext(
                "Hello Posts " . $request->getAttribute('post') . "!\n"
            );
        });
    });

@wpjscc wpjscc force-pushed the main branch 6 times, most recently from 4444cd7 to 8a662cc Compare October 18, 2023 12:43
This was referenced Oct 18, 2023
@wpjscc
Copy link
Author

wpjscc commented Oct 31, 2023

In the past few days of use, it has been very useful to control between different grouping APIs. I have compiled some tests, covering all the cases I can think of.

I think it's done!

Feel free to close the PR if you don't find it useful.

@SimonFrings
Copy link
Contributor

@wpjscc Thanks for opening this pull request and adding tests to confirm your changes 💪

As you may already know, we're currently working on a first version for Framework X which should go live tomorrow, see https://twitter.com/x_framework/status/1762499173830778907.

In preparation for this release, we've been brainstorming a ton of new features, including the possibility of supporting route groups. This is definitely a topic we'd like to see being part of Framework X!

While talking about route groups, we also came up with some plans for the current RouteHandler and the whole handler structure in general. Nothing set in stone yet, but the currently internal RouteHandler will probably be public at some point and renamed to Router. With that in mind, I'm not sure if we can start with an implementation on route groups today that may be completely changed in the future and thus force foreseeable BC breaks.

I really appreciate the time and thoughts you put into this (I really do!) and I invite you to keep using your implementation for the time being. However, I'm not sure if merging this is the right decision at the moment with the other plans we have in place.

I'll close this for ticket for now, but we'll definitely revisit this in an upcoming release! 👍️

@SimonFrings SimonFrings closed this Mar 4, 2024
@SimonFrings SimonFrings added the new feature New feature or request label Mar 4, 2024
@wpjscc
Copy link
Author

wpjscc commented Aug 24, 2024

Route Hello, I have extracted a composer package, utilizing it in a manner similar to Laravel's routing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants