wimdeblauwe / htmx-spring-boot

Spring Boot and Thymeleaf helpers for working with htmx
Apache License 2.0
521 stars 49 forks source link

Add support HtmxResponse that isn't a direct return type #127

Closed checketts closed 1 week ago

checketts commented 3 months ago

I've working with JTE as my templating engine recently and been very impressed. The templates are statically compiled and work very well with this library.

However there is an advanced use case where the Gradle or maven plugin generates an interface that represents your templates, complete with parameters.

To leverage these templates I'm returning a JteModel in my controller methods, that isn't compatible with this library. I would like to propose a few solutions.

1- Support subclassing HtmxResponse. I might be able to leverage that so my JteModel provides the right data. 2- Support making HtmxResponse an injectable parameter, similar to how you can set the response code when injecting HttpServletResponse. 3- Support a 'provider' approach for HtmxResponse. This means my response can implement an interface and provide an HtmxResponse, allowing the builder to remain the same but making the return type more flexible.

I'll start on a PR for option 3. But I'm interested in feedback on any and all of these options.

checketts commented 3 months ago

Reporting back: Option 3 won't work. Spring selects only one handler. That also means that option 1 won't work.

Perhaps option 2 will work.

checketts commented 2 months ago

@wimdeblauwe How do you feel about PR #128 ?

It is working really well for me in my 3 projects that I've migrated to JTE

wimdeblauwe commented 2 months ago

Hi @checketts, sorry I forgot about this. If you can just add some documentation, we can merge it and I will create a new release for you.

checketts commented 2 months ago

I just pulled this into a project and it is working great!

xhaggi commented 4 weeks ago

To leverage these templates I'm returning a JteModel in my controller methods, that isn't compatible with this library. I would like to propose a few solutions.

@checketts Do you have a special implementation in your projects to handle JteModel as return type?

checketts commented 3 weeks ago

@xhaggi Yes. I'll try to upload a sample project (and hopefully push upstream into the JTE Spring module)

checketts commented 3 weeks ago

Returning a JTE Model with Spring is not present in the main project, yet the concept aligns with the expected usecase. This isn't too surprising since the primary JTE committer doesn't use Spring.

I'm new to these internals, so I'm not familiar with how #137 might be impacted with the HtmxResponse being in the model instead of the return value. I haven't used flash attributes for a very long time (nor redirecting).

My primary usecase is having a programatic way to set HTMX headers following the same pattern you might use with HttpServletResponse. In Spring you typically inject the HttpServletResponse to do that. Injecting a HtmxResponse for the same pattern feel right to me.

However as long as injecting the HtmxResponse is supported, we don't need it to be in the Model. It could be a response attribute perhaps. Also I think it could make sense to throw son sort of Exception if the view is set on the HtmxResponse in that usecase and document that limitation.

xhaggi commented 3 weeks ago

In Spring you typically inject the HttpServletResponse to do that. Injecting a HtmxResponse for the same pattern feel right to me.

I agree with that. This gives me the idea that maybe we should remove the support for adding views from HtmxResponse and rather move this to a class HtmxView. We only need this view if we want to specify several views. In cases where only one view is needed, HtmxView would not be required as a return value.

Something like

Single view

String user(HtmxResponse res) {
  res.trigger("mytrigger");
  return "myview";
}

Multiple views

HtmxView user(HtmxResponse res) {
  res.trigger("mytrigger");
  var view = new HtmxView();
  view.add("myfragment1");
  view.add("myfragment2");
  return view;
}

This would bring the implementation more in line with Spring. @wimdeblauwe what do you think?

wimdeblauwe commented 3 weeks ago

Spring Framework 6.2 (Which is due November 2024) will support multiple fragments: https://docs.spring.io/spring-framework/reference/6.2/web/webmvc-view/mvc-fragments.html#page-title Maybe we should take that in account? I would be ok with releasing a 4.0 that needs 6.2 (When it is released and a compatible Spring Boot version is available obviously).

xhaggi commented 3 weeks ago

Good to know. This will make things much easier in the future. How about introducing HtmxView and marking the ability to use HtmxResponse as a return type as deprecated for the next version of this library (3.6.0). This way we could already adapt the documentation to this now and later in 4.0.0 we can remove the view stuff from HtmxResponse and mark HtmxView as deprecated in favor of returning a list of views (HTML fragments).

wimdeblauwe commented 2 weeks ago

Would we have HtmxView as return type with multiple views in there? Or go for a List<HtmxView> if you want OOB swaps? Does HtmxView also have methods to set model attributes?

So if I understand it correctly, we want to make HtmxResponse more like HttpServletResponse where you add it as a parameter to the controller method? And you call methods on the injected instance to influence the response. Seems like a good idea to me so it is more in line with how HttpServletResponse works.

We do have to make sure it can still work as a global error handler as documented in https://github.com/wimdeblauwe/htmx-spring-boot?tab=readme-ov-file#error-handlers

checketts commented 2 weeks ago

I favor List<HtmxView> It may not even be needed if Spring supports multiple views automatically.

xhaggi commented 2 weeks ago

@wimdeblauwe You are right. I'm already working on it and it's almost finished.

I favor List It may not even be needed if Spring supports multiple views automatically.

I need to check if this is possible. At the moment I am using HtmxView which contains a list of ModelAndView.

wimdeblauwe commented 2 weeks ago

I think it is ok to use HtmxView for now. It is basically just a wrapper for the List that it can become in the future and then maybe we don't need HtmxView anymore.

xhaggi commented 1 week ago

Can we close this issue?

checketts commented 1 week ago

I haven't tried it but if #140 fully addresses this then I'm fine closing it