Skip to content

Commit

Permalink
fix: Initialize the route only once when using eager server load (#16498
Browse files Browse the repository at this point in the history
)

Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
  • Loading branch information
Artur- authored Apr 4, 2023
1 parent 25a36a3 commit 09ed805
Show file tree
Hide file tree
Showing 10 changed files with 208 additions and 95 deletions.
19 changes: 11 additions & 8 deletions flow-client/src/main/frontend/Flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,6 @@ export class Flow {

// Call server side to navigate to the given route
flowRoot.$server.connectClient(
this.container.localName,
this.container.id,
this.getFlowRoutePath(ctx),
this.getFlowRouteQuery(ctx),
this.appShellTitle,
Expand Down Expand Up @@ -296,15 +294,20 @@ export class Flow {
await this.config.imports();
}

// Load flow-client module
const clientMod = await import('./FlowClient');
await this.flowInitClient(clientMod);

// we use a custom tag for the flow app container
const tag = `flow-container-${appId.toLowerCase()}`;
this.container = document.createElement(tag);
const serverCreatedContainer = document.querySelector(tag);
if (serverCreatedContainer) {
this.container = serverCreatedContainer as HTMLElement;
} else {
this.container = document.createElement(tag);
this.container.id = appId;
}
flowRoot.$[appId] = this.container;
this.container.id = appId;

// Load flow-client module
const clientMod = await import('./FlowClient');
await this.flowInitClient(clientMod);

// hide flow progress indicator
this.loadingFinished();
Expand Down
12 changes: 5 additions & 7 deletions flow-client/src/test/frontend/FlowTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ describe('Flow', () => {
if (indicator) {
indicator.remove();
}
Array.from(document.body.children)
.filter((e) => e.tagName.toLowerCase().startsWith('flow-container'))
.forEach((e) => e.remove());

$wnd.addEventListener = (type, listener) => {
listeners.push({ type: type, listener: listener });
Expand Down Expand Up @@ -704,18 +707,13 @@ function stubServerRemoteFunction(
flowRoot.$server = {
timers: [],

connectClient: (localName: string, elemId: string, route: string) => {
expect(localName).not.to.be.undefined;
expect(elemId).not.to.be.undefined;
connectClient: (route: string) => {
expect(route).not.to.be.undefined;
if (routeRegex) {
expect(route).to.match(routeRegex);
}

expect(elemId).to.equal(id);
expect(localName).to.equal(`flow-container-${elemId.toLowerCase()}`);

container = flowRoot.$[elemId];
container = flowRoot.$[id];

expect(container).not.to.be.undefined;
expect(container.serverConnected).not.to.be.undefined;
Expand Down
42 changes: 19 additions & 23 deletions flow-server/src/main/java/com/vaadin/flow/component/UI.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ public class UI extends Component

private static final String NULL_LISTENER = "Listener can not be 'null'";

private static final Pattern APP_ID_REPLACE_PATTERN = Pattern
.compile("-\\d+$");

/**
* The id of this UI, used to find the server side instance of the UI form
* which a request originates. A negative value indicates that the UI id has
Expand Down Expand Up @@ -262,8 +259,15 @@ public void doInit(VaadinRequest request, int uiId, String appId) {
}
this.uiId = uiId;

appId = APP_ID_REPLACE_PATTERN.matcher(appId).replaceAll("");
getInternals().setAppId(appId);
getInternals().setFullAppId(appId);

// Create flow reference for the client outlet element
wrapperElement = new Element(getInternals().getContainerTag());

// Connect server with client
getElement().getStateProvider().appendVirtualChild(
getElement().getNode(), wrapperElement,
NodeProperties.INJECT_BY_ID, appId);

// Add any dependencies from the UI class
getInternals().addComponentDependencies(getClass());
Expand Down Expand Up @@ -1637,6 +1641,8 @@ public Stream<Component> getChildren() {

private String forwardToClientUrl = null;

private boolean firstNavigation = true;

/**
* Gets the new forward url.
*
Expand All @@ -1650,10 +1656,6 @@ public String getForwardToClientUrl() {
* Connect a client with the server side UI. This method is invoked each
* time client router navigates to a server route.
*
* @param clientElementTag
* client side element tag
* @param clientElementId
* client side element id
* @param flowRoutePath
* flow route path that should be attached to the client element
* @param flowRouteQuery
Expand All @@ -1667,9 +1669,8 @@ public String getForwardToClientUrl() {
*/
@ClientCallable
@AllowInert
public void connectClient(String clientElementTag, String clientElementId,
String flowRoutePath, String flowRouteQuery, String appShellTitle,
JsonValue historyState, String trigger) {
public void connectClient(String flowRoutePath, String flowRouteQuery,
String appShellTitle, JsonValue historyState, String trigger) {

if (appShellTitle != null && !appShellTitle.isEmpty()) {
getInternals().setAppShellTitle(appShellTitle);
Expand All @@ -1692,21 +1693,16 @@ public void connectClient(String clientElementTag, String clientElementId,
} else {
navigationTrigger = NavigationTrigger.HISTORY;
}
if (wrapperElement == null) {
// Create flow reference for the client outlet element
wrapperElement = new Element(clientElementTag);

// Connect server with client
getElement().getStateProvider().appendVirtualChild(
getElement().getNode(), wrapperElement,
NodeProperties.INJECT_BY_ID, clientElementId);

if (firstNavigation) {
firstNavigation = false;
getPage().getHistory().setHistoryStateChangeHandler(
event -> renderViewForRoute(event.getLocation(),
event.getTrigger()));

// Render the flow view that the user wants to navigate to.
renderViewForRoute(location, navigationTrigger);
if (getInternals().getActiveRouterTargetsChain().isEmpty()) {
// Render the route unless it was rendered eagerly
renderViewForRoute(location, navigationTrigger);
}
} else {
History.HistoryStateChangeHandler handler = getPage().getHistory()
.getHistoryStateChangeHandler();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,18 @@
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.ComponentUtil;
import com.vaadin.flow.component.HasElement;
Expand Down Expand Up @@ -75,9 +80,6 @@
import com.vaadin.flow.shared.Registration;
import com.vaadin.flow.shared.communication.PushMode;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Holds UI-specific methods and data which are intended for internal use by the
* framework.
Expand All @@ -90,6 +92,9 @@
*/
public class UIInternals implements Serializable {

private static final Pattern APP_ID_REPLACE_PATTERN = Pattern
.compile("-\\d+$");

/**
* A {@link Page#executeJs(String, Serializable...)} invocation that has not
* yet been sent to the client.
Expand Down Expand Up @@ -200,6 +205,8 @@ public List<Object> getParameters() {

private String appId;

private String fullAppId;

private Component activeDragSourceComponent;

private ExtendedClientDetails extendedClientDetails = null;
Expand Down Expand Up @@ -983,11 +990,13 @@ public void setContinueNavigationAction(
* Sets the application id tied with this UI. Different applications in the
* same page have different unique ids.
*
* @param appId
* the id of the application tied with this UI
* @param fullAppId
* the (full, not stripped) id of the application tied with this
* UI
*/
public void setAppId(String appId) {
this.appId = appId;
public void setFullAppId(String fullAppId) {
this.fullAppId = fullAppId;
this.appId = APP_ID_REPLACE_PATTERN.matcher(fullAppId).replaceAll("");
}

/**
Expand All @@ -1000,6 +1009,17 @@ public String getAppId() {
return appId;
}

/**
* Gets the full app id, which funnily enough is not the same as appId. This
* really should be removed but not right now. Don't use this method, it
* will be gone.
*
* @return the full app id
*/
public String getFullAppId() {
return fullAppId;
}

/**
* Gets the router used for navigating in this UI, if the router was active
* when this UI was initialized.
Expand Down Expand Up @@ -1215,4 +1235,9 @@ private void removeChildrenContentFromRouterLayout(
oldContent = oldChildren.get(oldContent);
}
}

public String getContainerTag() {
return "flow-container-" + getFullAppId().toLowerCase(Locale.ENGLISH);

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,18 @@ public boolean synchronizedHandleRequest(VaadinSession session,
if (service.getBootstrapInitialPredicate()
.includeInitialUidl(request)) {
includeInitialUidl(initialJson, session, request, response);

UI ui = UI.getCurrent();
var flowContainerElement = new Element(
ui.getInternals().getContainerTag());
flowContainerElement.attr("id", ui.getInternals().getAppId());
Elements outlet = indexDocument.body().select("#outlet");
if (!outlet.isEmpty()) {
outlet.first().appendChild(flowContainerElement);
} else {
indexDocument.body().appendChild(flowContainerElement);
}
indexHtmlResponse = new IndexHtmlResponse(request, response,
indexDocument, UI.getCurrent());
indexDocument, ui);
} else {
indexHtmlResponse = new IndexHtmlResponse(request, response,
indexDocument);
Expand Down
Loading

0 comments on commit 09ed805

Please sign in to comment.