geowarin/boot-react

Missing resources resolution

Opalo opened this issue · 6 comments

Opalo commented

Hi,

First of all, great job! Thanks!

I've question regarding missing resources resolution, SinglePageAppConfig class in particular.

First of all, please replace orElseGet(null) with orElse(null). orElseGet will throw NPE when null is passed whereas orElse will return null.

Secondly, you wrote that if a requested view is not found it should be redirected to index.html. It will not work. some.html is handled - according to extension - then it's not found and null will be returned - this is how resolve method logic works now.

geowarin commented

Hi @Opalo!

Thanks for your awesome remarks. Yes SinglePageAppConfig is a bit fragile.
Moreover, it's not tested. Would you mind opening a PR to address the issues you mentioned?
That would be a great help!

Thanks 😄

Opalo commented

@geowarin, will try. Could you please specify the business logic for this resolver? As far as I understood from the comment (on the top of the class) it should redirect all *.html resources that were not found to index.html site. If so, why handledExtensions also have css, js and other files? I may try to provide my implementation of such resolver but must know how it should work.

  • 👍1
geowarin commented

The idea is to redirect all non-resource (resources are like css, images, etc.) requests to the index.html.
A GET request for an html page should not be resolved by spring but by React-router on the client side.

I don't remember exactly where I got the idea of whitelisting the resources extensions for the fallback but I do not think it is the best way to go anymore.

The best implementation that I found is what the history API fallback used in server.js does.

Algorithm:

  • If the HTTP verb is GET
  • And the preferred value for the Accept header is not application/json
  • And the values of the Accept headers contain text/html or */*
  • And the requested pahtname (what's after the hostname) does not contain a dot
  • Redirect to index.html
  • Otherwise, delegate to the existing resolver chain.

I think I tried to be smart and handle multiple html files but that's clearly over-engineered when I look back at it.

PS: The react-router docs also suggest an implementation.

Opalo commented

@geowarin, thanks for the input. The algorithm you specified is possible to implement in resolveResource method. There's also a second method called resolveUrlPath which need to be implemented in this interface - how it should be implemented then? Maybe my questions are trivial however I consider myself as a spring beginner.

geowarin commented

I don't know much about resource resolution in Spring either.
I'll try to take a look and make things cleaner when I have time.

If you are still motivated, trust your instincts and we'll review any PR your send our way to try and catch potential mistakes;

geowarin commented

Closing in favor of #30