Various Turbo-related improvements

[Kreyu]

Based on the discussion in #180

Configurable turbo frame identifier via option

I'm thinking of adding an option to the base DataTableType that allows setting a turbo frame identifier:

$this->createDataTable(ProductDataTable::class, $query, [
    'turbo_frame_id' => 'custom_turbo_frame_id',
]);

By default, that option would be null.

Turbo frame identifier in data table config and its builder

Following methods could be added to the data table config:

• DataTableConfigInterface::getTurboFrameIdentifier()
• DataTableConfigBuilderInterface::setTurboFrameIdentifier(string $turboFrameIdentifier)

The base DataTableType could use the setTurboFrameIdentifier to set the value from turbo_frame_id option. If that option equals null, default to kreyu_data_table_ + name of the data table.

Changes in request handler

In HttpFoundationRequestHandler, we could check whether the request has Turbo-Frame header:

• if it doesn't, then it shouldn't change anything
• if it does, then we should handle the request only if the header value equals to:
  $dataTable->getConfig()->getTurboFrameIdentifier()

This would improve DX a lot, suggested by @alexandre-castelain in #180 (comment)

We could add new method to the RequestHandlerInterface:

public function isRequestFromTurboFrame(DataTableInterface $dataTable, mixed $request = null): bool;

This method will return whether the request contains the Turbo-Frame header equal to $dataTable->getConfig()->getTurboFrameIdentifier(), and it should be implemented in the HttpFoundationRequestHandler.

Changes in DataTableInterface

We could add new method to the DataTableInterface:

public function isRequestFromTurboFrame(mixed $request): bool;

This method would simply call the configured request handler's isRequestFromTurboFrame method.

Changes in Twig

In Twig, we could introduce a data_table_turbo_frame_id method, that simply retrieves an instance of DataTableView and returns value of its turbo_frame_id variable, for better compatibility in the future.

The base.html.twig theme should use the data_table_turbo_frame_id function.

Additional template should be introduced, that simply renders the data_table variable for streamed response, explained below:

{# @KreyuDataTable/streamed_data_table.html.twig #}
{{ data_table(data_table) }}

Changes in traits

Probably another trait that works like a DataTableFactoryAwareTrait, but with renderDataTableTurboStream method (I'm open for better naming):

trait DataTableFactoryAwareControllerTrait
{
    public function renderDataTableTurboStream(DataTableInterface $dataTable): TurboStreamResponse
    {
        $renderedDataTable = $this->render('@KreyuDataTable/streamed_data_table.html.twig', [
            'data_table' => $dataTable->createView(),
        ]);

        return new TurboStreamResponse()->replace("#".$dataTable->getConfig()->getTurboFrameIdentifier(), $renderedDataTable);
    }
}

[Kreyu]

@alexandre-castelain based on your example in #180 (comment), how would we handle the streamDataTable method in DataTableFactoryAwareTrait? It needs template, block name and context to render specific block - so it would work exactly like renderBlock?

[alexandre-castelain]

This is not tested, right ? But here's my idea : https://gist.github.com/alexandre-castelain/de62a66a455334152f2371fab562a9a6

As the kreyu_data_table already contains the turbo-frame id, Turbo will change the previous one by the new one if I understand well how this works.

So we would return a simple template that only contains the dataTable itself.

As you said, I'm not sure we should use the Trait for this.
At the moment, with the tool I use, it's the DataTable itself that builds the response, so we can do $dataTable->getResponse(). We don't need to inject another service to handle this behaviour, and that's pretty cool.

[FluffyDiscord]

The data table trait needs to set specific headers. You want to return either custom response, or use what symfony suggests.

$renderedDataTable = $this->render('@kreyu_data_table/streamed_data_table.html.twig', [
            'data_table' => $dataTable,
        ]);

return new TurboStreamResponse()->replace("#".$extension->getDataTableName($dataTableView), $renderedDataTable);

[Kreyu]

Thanks for the suggestions, it keeps shaping up better and better.

Another small DX change, which unfortunately would be a breaking change, is to make DataTableInterface::handleRequest fluent by returning the DataTableInterface, instead of void. This would make this possible:

// before
$products = $this->createDataTable(...);
$products->handleRequest($request);
$categories = $this->createDataTable(...);
$categories->handleRequest($request);

// after
$products = $this->createDataTable(...)->handleRequest($request);  
$categories = $this->createDataTable(...)->handleRequest($request);  

Not that big of a deal though 🙂

[FluffyDiscord]

I think this should be kept as is, to not deviate too much from Symfony Forms. I love that I feel at home with this package and these minor adjustments would throw me off, honestly.

[RafaelKr]

Another small DX change, which unfortunately would be a breaking change, is to make DataTableInterface::handleRequest fluent by returning the DataTableInterface, instead of void. This would make this possible:

// before
$products = $this->createDataTable(...);
$products->handleRequest($request);
$categories = $this->createDataTable(...);
$categories->handleRequest($request);

// after
$products = $this->createDataTable(...)->handleRequest($request);  
$categories = $this->createDataTable(...)->handleRequest($request);  

Not that big of a deal though 🙂

I really would like that. Maybe suggesting this to Symfony Forms and add it to this package if they add it would be nice? 😁

[Kreyu]

Correct me if I'm wrong, but I see a small issue with all that logic. Let's assume that I have following controller:

class UserController extends AbstractController
{
    use DataTableControllerTrait;

    public function show(Request $request, User $user)
    {
        $userPosts = $this->createDataTable(PostDataTableType::class, ...);
        $userComments = $this->createDataTable(CommentDataTableType::class, ...);
        
        $userPosts->handleRequest($request);
        $userComments->handleRequest($request);
        
        return $this->render('user/show.html.twig', [
            'user_posts' => $userPosts->createView(),
            'user_comments' => $userComments->createView(),
        ]);
    }
}

Now, let's say we modify request handler to execute only if Turbo-Frame header equals to a data table turbo frame identifier:

class HttpFoundationRequestHandler implements RequestHandlerInterface
{
    public function handle(DataTableInterface $dataTable, mixed $request = null): void
    {
        $turboFrame = $request->headers->get('Turbo-Frame');

        if (null !== $turboFrame && $turboFrame !== $dataTable->getConfig()->getTurboFrameIdentifier()) {
            return;
        }

        // Current handle() logic
    }
}

If user interacts with user_posts, the user_comments state won't be updated based on the request. Therefore, in this case, Turbo will replace the user_comments turbo frame without any filters, pagination etc. Unless we explicitly add:

if ($userPosts->isRequestFromTurboFrame()) {
    return $this->renderDataTableTurboStream($userPosts);
}

if ($userComments->isRequestFromTurboFrame()) {
    return $this->renderDataTableTurboStream($userComments);
}

I don't see a way to make it compatible. However, my last suggestion was to leave the request handler as-is, and call handleRequest like this:

class UserController extends AbstractController
{
    use DataTableControllerTrait;

    public function show(Request $request, User $user)
    {
        $userPosts = $this->createDataTable(PostDataTableType::class, ...);
        $userComments = $this->createDataTable(CommentDataTableType::class, ...);
        
        if ($userPosts->isRequestFromTurboFrame()) {
            $userPosts->handleRequest($request);

            return $this->renderDataTableTurboStream($userPosts);
        }
        
        if ($userComments->isRequestFromTurboFrame()) {
            $userComments->handleRequest($request);

            return $this->renderDataTableTurboStream($userComments);
        }

        $userPosts->handleRequest($request);
        $userComments->handleRequest($request);
        
        return $this->render('user/show.html.twig', [
            'user_posts' => $userPosts->createView(),
            'user_comments' => $userComments->createView(),
        ]);
    }
}

Maybe we could pass the request to the renderDataTableTurboStream method (that new shiny method from trait, naming TBD), but it kinda hides the usage of handleRequest...

[FluffyDiscord]

What about

class UserController extends AbstractController
{
    use DataTableControllerTrait;

    public function show(Request $request, User $user)
    {
        $userPosts = $this->createDataTable(PostDataTableType::class, ...);
        $userComments = $this->createDataTable(CommentDataTableType::class, ...);
        
        if ($userPosts->handledTurboStreamRequest($request)) {
            return $this->renderDataTableTurboStream($userPosts);
        }
        
        if ($userComments->handledTurboStreamRequest($request)) {
            return $this->renderDataTableTurboStream($userComments);
        }

        $userPosts->handleRequest($request);
        $userComments->handleRequest($request);
        
        return $this->render('user/show.html.twig', [
            'user_posts' => $userPosts->createView(),
            'user_comments' => $userComments->createView(),
        ]);
    }
}

where

function handledTurboStreamRequest(Request $request): bool
{
    if($this->isRequestFromTurboFrame($request)) {
        $this->handleRequest($request);

        return true;
    }
  
    return false;
}

This feels nicer, and you definitely want to pass request to the isRequestFromTurboFrame() and not use RequestStack.

[Kreyu]

Yeah, you're right, I meant to pass the request object to isRequestFromTurboFrame.

handledTurboStreamRequest seems nicer, but it kinda confuses me a bit, since that naming doesn't really suggest that it is doing two things at once. But I don't have a better suggestion for now.

[alexandre-castelain]

Hi @Kreyu ! Hi @FluffyDiscord !

I tried to implement what we discussed last week. You can find it here : https://github.com/alexandre-castelain/data-table-bundle/tree/async-and-turbo

Several information :

• This is possible to render a DataTable as asynchronous : the dataTable won't be loaded on page load but after through AJAX. This can be activated on any DT through the option

//UserDataTable.php
public function configureOptions(OptionsResolver $resolver): void
{
    $resolver->setDefaults([
        'is_asynchronous' => true,
    ]);
}

• To improve performance, as we discussed, you can return only the DT content

//MyController.php
public function index(
    ProductRepository $productRepository, Request $request,
): Response
{
    $query = $productRepository->createQueryBuilder('product');
    $dataTableProduct = $this->createDataTable(ProductDataTableType::class, $query);
    $dataTableProduct->handleRequest($request);

    if ($dataTableProduct->isRequestFromTurboFrame()) {
        return $this->renderDataTableTurboFrameResponse($dataTableProduct);
    }

    return $this->render('home/index.html.twig', [
        'products' => $dataTableProduct->createView(),
    ]);
}

Other consideration :

• The TurboStreamResponse is deprecated as you can see here (in the tip part): https://symfonycasts.com/screencast/turbo/turbo-stream
• Anyway, this comes from the TurboBundle that is not needed in our case. I dont understand what's the $request->setRequestFormat(TurboBundle::STREAM_FORMAT); is usefull to.
• We need to have the URL to refresh the DT, at the moment, this is hardcoded but there's probably a better way to do it : https://github.com/alexandre-castelain/data-table-bundle/blob/async-and-turbo/src/Resources/views/themes/base.html.twig#L12
• This is a POC ofc, to try it and see if this can work the way I had it in mind

What do you think about it ?

[Kreyu]

Anyway, this comes from the TurboBundle that is not needed in our case. I dont understand what's the $request->setRequestFormat(TurboBundle::STREAM_FORMAT); is usefull to.

If I recall correctly, it sets the media type to text/vnd.turbo-stream.html, which is handled differently in Turbo:

• https://turbo.hotwired.dev/handbook/streams#streaming-from-http-responses
• https://symfony.com/bundles/ux-turbo/current/index.html#coming-alive-with-turbo-streams

The TurboStreamResponse is deprecated as you can see here (in the tip part): https://symfonycasts.com/screencast/turbo/turbo-stream

It's not deprecated, looks like it's just alternative way to do it than setRequestFormat.

Other points I thought of:

• Isn't is_asynchronous and src on turbo frame adding another feature - lazy loading? Data tables are kiiinda asynchronous by default thanks to being wrapped in turbo frame
• I think isRequestFromTurboFrame should require Request object as an argument and call its request handler to return bool, instead of using setIsRequestFromTurboFrame and holding it in data table
• Your HttpFoundationRequestHandler::handle will probably break for requests without turbo frame header (see compatibility issues in my comment above Various Turbo-related improvements #196 (comment))

[alexandre-castelain]

It's not deprecated, looks like it's just alternative way to do it than setRequestFormat.

Alright, the Trait now need the Request to update the RequestFormat as you can see here.

I think isRequestFromTurboFrame should require Request object as an argument and call its request handler to return bool, instead of using setIsRequestFromTurboFrame and holding it in data table

I updated to code to process it from the Request object. The downside is that, we need a $request object to know if we are currently in a TurboFrame or not.

Your HttpFoundationRequestHandler::handle will probably break for requests without turbo frame header (see compatibility issues in my comment above

Maybe I'm wrong, but I tested it before and it does not break, because I first check if we're in a TurboFrame context or not

public function handle(DataTableInterface $dataTable, mixed $request = null): void 
{
    //...
    if (!$dataTable->isRequestFromTurboFrame($request)) {
          return;
    }
    $this->filter($dataTable, $request);
    $this->sort($dataTable, $request);
    $this->personalize($dataTable, $request);
    $this->paginate($dataTable, $request);
    $this->export($dataTable, $request);
    //...
}

So if the request is not coming from a TurboFrame, the handle function works as usual.

The only thing that does not work properly when not in a Turbo context is the is_asynchronous part. As expected, because this is turbo that does the request and fetches the data. But I don't think this is really a problem.

With these update, here is what a controller looks like :

public function index(
    ProductRepository $productRepository, 
    Request $request,
): Response
{
    $query = $productRepository->createQueryBuilder('product');
    $dataTableProduct = $this->createDataTable(ProductDataTableType::class, $query);
    $dataTableProduct->handleRequest($request);

    if ($dataTableProduct->isRequestFromTurboFrame($request)) {
        return $this->renderDataTableTurboFrameResponse($dataTableProduct, $request);
    }

    return $this->render('home/index.html.twig', [
        'products' => $dataTableProduct->createView(),
    ]);
}

I don't really like the fact that we need to inject the request everywhere. In a Form context, there's not need to do that, we just call ->isValid() && ->isSubmitted() and the form knows if he's valid or not. That's why I did like that before.

[alexandre-castelain]

Hey @Kreyu, any news about this ?

[Kreyu]

Not yet, sorry, wasn't doing anything related to it. It's a bit low on my priority list, but I'll try my best to get back to you guys with some conclusion 😄

[alexandre-castelain]

No problem, I understand :)
Have fun doing it !

I'm available to help if needed, don't hesitate to ping me, even if it's not related to this topic.