Skip to content

Bugfix/layout remount #2389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion MIGRATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Please open an issue or a pull request if you feel this doc is incomplete.

## NEXT

- Update React-Router to v5+. Versions must match exactly between react-router and react-router-dom `npm i --save-exact [email protected] [email protected]`. Note: v4 to v5 is supposed to be a minor update so there are not many breaking changes.
- Optionally update react-router-bootstrap to v0.25.0 `npm i --save-exact [email protected]`

## From 1.13.5 to 1.14

- See migration article from [Vulcan Blog](https://blog.vulcanjs.org/)
Expand All @@ -21,7 +24,6 @@ Please open an issue or a pull request if you feel this doc is incomplete.
- `npm install apollo-utilities` (to run tests)
- Replace `Users.getViewableFields` by `Users.getReadableProjection`


## From 1.13.2 to 1.13.3

- Update React to a version over 16.8 (and under 17 which will bring breaking changes) to access hooks
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@
"react-markdown": "^3.1.4",
"react-places-autocomplete": "^5.0.0",
"react-redux": "^5.0.6",
"react-router": "^4.3.1",
"react-router-bootstrap": "^0.24.4",
"react-router-dom": "^4.3.1",
"react-router": "5.0.1",
"react-router-bootstrap": "0.25.0",
"react-router-dom": "5.0.1",
"react-router-scroll": "^0.4.4",
"react-stripe-checkout": "^2.4.0",
"recompose": "^0.26.0",
Expand Down
127 changes: 76 additions & 51 deletions packages/vulcan-core/lib/modules/components/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,37 +21,82 @@ import { Switch, Route } from 'react-router-dom';
import { withRouter } from 'react-router';
import MessageContext from '../messages.js';

// see https://stackoverflow.com/questions/42862028/react-router-v4-with-multiple-layouts
const RouteWithLayout = ({ layoutComponent, layoutName, component, currentRoute, ...rest }) => {
// if defined, use ErrorCatcher component to wrap layout contents
const ErrorCatcher = Components.ErrorCatcher ? Components.ErrorCatcher : Components.Dummy;
const getRouteLayoutName = route => {
const { layoutComponent, layoutName } = route;
if (layoutComponent && (!layoutComponent.displayName || layoutName)) {
throw new Error(
'Adding route with layoutComponent but without displayName or layoutName. ' +
'An unique name is mandatory to group routes per layout.' +
'Please add a displayName to your layoutComponent or specify the layoutName property. ' +
`Route: ${route.name} ${route.path}`
);
}
const finalLayoutName = layoutComponent
? layoutComponent.displayName || layoutName
: layoutName || 'Layout';
return finalLayoutName;
};

const groupRoutesPerLayout = routes =>
Object.keys(routes).reduce((perLayout, routeName) => {
const route = routes[routeName];
const layoutName = getRouteLayoutName(route);
if (!perLayout[layoutName]) {
perLayout[layoutName] = {
routes: [],
layoutComponent: route.layoutComponent || Components[layoutName],
};
}
perLayout[layoutName].routes.push(route);
return perLayout;
}, {});

const RouteSwitch = ({ routes, siteData }) => {
const routesPerLayout = groupRoutesPerLayout(routes);
const layoutNames = Object.keys(routesPerLayout);
const ErrorCatcher = Components.ErrorCatcher ? Components.ErrorCatcher : Components.Dummy;
return (
<Route
// NOTE: Switch ignores the "exact" prop of components that
// are not its direct children
// Since the render tree is now Switch > RouteWithLayout > Route
// (instead of just Switch > Route), we must write <RouteWithLayout exact ... />
//exact
{...rest}
render={props => {
const layoutProps = { ...props, currentRoute };
const childComponentProps = { ...props, currentRoute };
// Use layoutComponent, or else registered layout component; or else default layout
const layout = layoutComponent
? layoutComponent
: layoutName
? Components[layoutName]
: Components.Layout;
const children = (
<ErrorCatcher>
<Components.RouterHook currentRoute={currentRoute} />
{React.createElement(component, childComponentProps)}
</ErrorCatcher>
<Switch>
{layoutNames.map(layoutName => {
const { routes, layoutComponent } = routesPerLayout[layoutName];
const Layout = layoutComponent;
return (
<Route key={layoutName} exact path={routes.map(r => r.path)}>
<Layout>
{routes.map(route => {
const {
name,
component: routeComponent,
componentName,
...otherRouteParams
} = route;
const component = routeComponent || Components[componentName];
return (
<Route
key={name}
exact
render={props => {
const childComponentProps = { ...props, currentRoute: route };
return (
<ErrorCatcher>
<Components.RouterHook currentRoute={route} />
{React.createElement(component, childComponentProps)}
</ErrorCatcher>
);
}}
{...otherRouteParams}
/>
);
})}
</Layout>
</Route>
);
return React.createElement(layout, layoutProps, children);
}}
/>
})}
<Components.Layout>
<Route siteData={siteData} currentRoute={{ name: '404' }} component={Components.Error404} />
</Components.Layout>
{/* <Route component={Components.Error404} /> */}
</Switch>
);
};

Expand Down Expand Up @@ -183,11 +228,10 @@ class App extends PureComponent {
}

render() {
const routeNames = Object.keys(Routes);
const { flash } = this;
const { messages } = this.state;
const { siteData } = this.props;
//const LayoutComponent = currentRoute.layoutName ? Components[currentRoute.layoutName] : Components.Layout;

return (
<IntlProvider
locale={this.getLocale()}
Expand All @@ -199,27 +243,8 @@ class App extends PureComponent {
<Components.HeadTags />
{this.props.currentUserLoading ? (
<Components.Loading />
) : routeNames.length ? (
<Switch>
{routeNames.map(key => (
// NOTE: if we want the exact props to be taken into account
// we have to pass it to the RouteWithLayout, not the underlying Route,
// because it is the direct child of Switch
<RouteWithLayout
exact
currentRoute={Routes[key]}
siteData={this.props.siteData}
key={key}
{...Routes[key]}
/>
))}
<RouteWithLayout
siteData={this.props.siteData}
currentRoute={{ name: '404' }}
component={Components.Error404}
/>
{/* <Route component={Components.Error404} /> */}
</Switch>
) : Object.keys(Routes).length ? (
<RouteSwitch routes={Routes} siteData={siteData} />
) : (
<Components.Welcome />
)}
Expand Down