search

Financial-Times / dotcom-page-kit

📰 Page Kit provides a high quality, well tested, and thoroughly documented set of tools for assembling and delivering FT.com based upon the best industry standards.
https://financial-times.github.io/dotcom-page-kit/
21 stars 6 forks source link

Wrapping HTML responses with the shell and layout components #524

Open i-like-robots opened 4 years ago

i-like-robots commented 4 years ago

I'm opening this issue to discuss how we should provide a package capable of neatly integrating the shell and layout components into our existing applications in order to "wrap" their HTML output with the shared page bootstrapping, metadata, core branding, and UI.

As initially discussed in #98 "preset" packages like this can be used to prevent applications implementing lots of divergent custom code.

Our current approach

The v0.1.0 n-ui to Page Kit migration guide instructs developers to add a callback argument to Express's response.render() method in order to capture the HTML output so that it can be used for a second render step. This is not too dissimilar to the the existing Handlebars layout mechanism it replaces.

The approach is "intentionally hacky" in order to expose how the parts fits together and it is relatively simple. However, it means Express arguments need to be passed around in addition to configuration data for the shell and layout components which may be confusing and tricky to mentally parse.

// server/controllers/lib/page-kit-wrapper.js
const React = require('react');
const ReactDOM = require('react-dom/server');
const { Shell } = require('@financial-times/dotcom-ui-shell');
const { Layout } = require('@financial-times/dotcom-ui-layout');

module.exports = ({ response, next, shellProps, layoutProps }) => {
    return (error, html) => {
        if (error) {
            return next(error);
        }

        const document = React.createElement(
            Shell,
            { ...shellProps },
            React.createElement(Layout, { ...layoutProps, contents: html })
        );

        response.send('<!DOCTYPE html>' + ReactDOM.renderToStaticMarkup(document));
    };
};

// server/controllers/page.js
const pageKitWrapper = require('./lib/page-kit-wrapper.js');

module.exports = (request, response, next) => {
    const templateData = {};

    const shellProps = {
        pageTitle: content.title,
    };

    const layoutProps = {
        navigationData: response.locals.navigation,
        headerOptions: { ...response.locals.anon }
    };

    const pageKitOptions = { request, response, next, shellProps, layoutProps };

    response.render('video.html', templateData, pageKitWrapper(pageKitOptions));
};
i-like-robots commented 4 years ago

My initial thoughts on this are that I would like where possible for UI components to not know anything about the request/response objects in order to be generic and reusable with any framework or tool.

IMO Express view engines are not especially useful, they are really responsible for three things:

  1. Find that the requested "view" file exists (the location of which is based upon some Express settings)
  2. Merge app.locals, response.locals, and the template data together
  3. Capture any rendering errors and call the fall-through function (next())

In a JSX world the first feature is not very useful because we can always use Node's built in module require() function. The second feature is a one liner now that we have Object.assign() and the spread operator. The third feature is quite useful but trivial to implement.

Given this I suspect it may be simpler to avoid the view engine paradigm and callbacks altogether and do something more generic along these lines:

// server/controllers/page.js
const handlebars = require('../lib/handlebars');
const pageKitPreset = require('@financial-times/dotcom-ui-preset');

module.exports = (request, response, next) => {
    const templateData = {};

    const shellProps = {
        pageTitle: content.title,
    };

    const layoutProps = {
        navigationData: response.locals.navigation,
        headerOptions: { ...response.locals.anon }
    };

    try {
        const html = handlebars.render('views/page.html', {
            ...app.locals,
            ...response.locals,
            ...templateData
        });

        response.send(pageKitPreset({ shellProps, layoutProps, html }));
    } catch (error) {
        next(error);
    }
};