vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
588 stars 164 forks source link

Strange behavior with History -> replaceState(state,location) when react is enabled #19580

Open tinostein opened 2 weeks ago

tinostein commented 2 weeks ago

Description of the bug

Since update to Vaadin 24.4.1 there is a strange behavoir when using replaceState() in History class. If I use the method to update the displayed URL (eg. history.replaceState(null,"http://localhost:8080/?test=test") , it results in opening the invalid url http://localhost:8080/http://localhost:8080/?test=test. If I set react.enabled=false, then it works as expected.

Expected behavior

Browser shows expected URL after history.replaceState call.

Minimal reproducible example

Use any Vaadin 24.4.1 project (eg. skeleton-starter-flow-spring.24).

Call method for updating url parameter, eg:

       Button button = new Button("Test - Add URL Parameter", e -> {
            updateUrlRequestParameter("test",textField.getValue());
        });

with:

public static void updateUrlRequestParameter(String key, String value) {
        Page page = UI.getCurrent().getPage();
        page.fetchCurrentURL(url -> {
            try {
                UriComponentsBuilder builder = UriComponentsBuilder.fromUri(url.toURI());
                UriComponents components = builder.build();
                MultiValueMap<String, String> parameters = components.getQueryParams();
                MultiValueMap<String, String> newParams = new LinkedMultiValueMap<>();
                newParams.putAll(parameters);
                if(newParams.get(key) != null) {
                    newParams.remove(key);
                }
                if(value != null) {
                    newParams.put(key, List.of(value));
                }
                String newUrl = builder.replaceQueryParams(newParams).build().toUriString();
                page.getHistory().replaceState(null, newUrl);
            } catch(Exception exception) {
                //handle exception
            }
        });
    }

Versions

mshabarov commented 1 week ago

Looks like a bug in Flow/React Router integration that is new in 24.4. react.enabled=false can be used indeed as a workaround for Flow apps unless this bug gets fixed.

caalador commented 1 week ago

ReplaceState requires the origin to be the same as for the current URL so the usual way is to just give the replaced part e.g. ?test=test instead of http://localhost:8080/?test=test

Also instead of going through the extra roundtrip to the client with page.fetchCurrentURL it would perhaps be clearer to just take the current location and update the queryParameters map (keeping the old ones or just replacing it all), so something like:

        UI ui = UI.getCurrent();
        Location activeViewLocation = ui.getActiveViewLocation();
        QueryParameters queryParameters = activeViewLocation.getQueryParameters()
                .merging("test", "test");
        ui.getPage().getHistory().replaceState(null, new Location(activeViewLocation.getPath(), queryParameters));

The direct call to the browser history.replaceState api removes the origin automatically, but react-router navigation navigates to the full given url.

stefanbrenner commented 1 week ago

We have another problem that may be related to this one:

We have a component with a route that implements BeforeLeaveObserver. Within this component we call replaceState on the history.

With the legacy Vaddin router the beforeLeave method is not called on the component whereas it is called with the React router.

@Route(value = "test/:child?")  
public class TestView extends VerticalLayout implements BeforeEnterObserver, BeforeLeaveObserver {  

    public TestView() {  
        add(new Button("Replace history state", click -> updatePathWithSelectedChild("child2")));  
    }  

    @Override  
    public void beforeEnter(BeforeEnterEvent event) {  
        add(new NativeLabel("beforeEnter Called"));  
    }  

    @Override  
    public void beforeLeave(BeforeLeaveEvent beforeLeaveEvent) {  
        add(new NativeLabel("beforeLeave Called"));  
    }  

    private void updatePathWithSelectedChild(String child) {  
        var page = UI.getCurrent().getPage();  
        page.fetchCurrentURL(url -> page.getHistory().replaceState(null, buildNewLocation(url, child)));  
    }  

    private Location buildNewLocation(URL url, String child) {  
        var path = url.getPath();  
        path = path.substring(path.indexOf("/test"), path.lastIndexOf("/") + 1) + child;  
        return new Location(path);  
    }  

}

Yet another difference: With the React router the URL is updated accordingly, with the legacy Vaadin router not!

mshabarov commented 3 days ago

@stefanbrenner thanks for your comment. Could you please make a new ticket for the described case. It would be easier to track these issues separately.

stefanbrenner commented 3 days ago

@stefanbrenner thanks for your comment. Could you please make a new ticket for the described case. It would be easier to track these issues separately.

see https://github.com/vaadin/flow/issues/19613