Spike: integrate existing CB into Volto

[nosnickid]

Integrating this into a volto addon.

Having some trouble with govuk-react-jsx again - the import problems - so I'm going to fork it and try to package it 'correctly' so that the server side rendering works.

[nosnickid]

Spent all day faffing with this. Distributing assets in node packages is a free for all; there's no canonical way to handle it. https://github.com/webpack/webpack/issues/7353 goes into this in some depth.

We have another fiddly element; we're trying to import() an image from a peer dependency. It doesn't make any sense to include the peer dependency image in the built govuk-react-jsx anyway.

I spent the day messing with alternative ways of packaging govuk-react-jsx, as a ES module instead of JS, but that was a bit of a rabbit hole that couldn't fix it.

To start from the top.

• We're using govuk-react-jsx which has an import statement for an image contained in govuk-frontend.
• If we use the packaged govuk-react-jsx available in npm, the ES6 source (using import) has been transpiled by babel into CommonJS (require())
• Because it is in node_modules in our project, the razzle webpack loaders assume it can be loaded directly and let the node cjs require() happen directly
• Ultimately this means node tries to parse the PNG as a JS file, and obviously fails.

I'm going to try one more thing, which is to add a webpack loader that will actually load the govuk-react-jsx module from node_modules. That should intercept the require() on the image.

[nosnickid]

Another long day of fighting razzle on this.

Razzles SSR is done in a express webserver in the server.jsx file in volto. Because it's jsx, that implies that webpack is intercepting the file load, passing it through babel to transpile jsx -> React.createElement calls, and then letting Node load the transpiled file.

If that is the case, I can't currently explain why, when it eventually loads govuk-react-jsx/.../Header.js, the require() statement in that file for the PNG isn't also intercepted by webpack, and handle by its file-loader.

[nosnickid]

Getting somewhere now. govuk-react-jsx is being treat as an external by webpack, which mean it gets loaded directly.

Hacking the externals to whitelist (ie, to treat it as not-external...) govuk-react-jsx fixes it.

So now I just need to patch the webpack config in the right way.

[nosnickid]

Finally got this fixed and merged to main in https://github.com/GSS-Cogs/dd-cms/pull/84

The trick was to replace the volto provided webpack config externals function with one that always treats govuk-react-jsx as not-external, so that webpack intercepts the image require calls and everything works as expected.

I've put together a basic volto add on and a volto block component that tries to use the chart builder in https://github.com/GSS-Cogs/dd-cms/tree/issue-63/volto-chart-builder-addon, and now passing this to Charles to resolve the problems that have arisen trying to import chart builder into volto.

[nosnickid]

Some progress on creating a volto addon for chart builder.

It's not working yet. The packaging of plotly and d3 isn't really compatible with server side rendering. What I've committed most recently works around a bunch of 'simple' loading resolution problems.

But now I'm starting to hit browser detection using window.navigator testing, which when it runs in the SSR, completely fails.

From plotly:

var match = FIREFOX_VERSION_REGEX.exec(window.navigator.userAgent);

Next step I guess is to start removing these things using the string-replace-loader. But it's piling hacks on top of hacks. It could be that we need a better way to ignore certain code paths for SSR. But that is quite an interesting thing to solve. I'll have a think over the weekend.

[charlesons]

@nosnickid , good one on the progress.

This thread looks like it could be relevant:

https://github.com/plotly/react-plotly.js/issues/21

[charlesons]

Basically looks like the fundamental issue is that plotly.js doesn't support SSR

"A point to note here, as of the time of writing this blog, ploty.js is a client-side rendering library and doesn't yet support server side rendering."

source: https://dev.to/dheerajmurali/building-a-responsive-chart-in-react-with-plotly-js-4on8

[nosnickid]

Ah ha. Ok, so I think the next step is to make chart builder detect when it's running server side (__SERVER__ constant I think) and conditionally load react-plotly, by using require() rather than import.

[charlesons]

Yes sounds right.

Another potential solution is to use the webpack feature for separate imports for server and browser suggested by @nhimhobao:

https://github.com/jaredpalmer/razzle/issues/606

[nosnickid]

Hrm, with react plotly being used so deeply in the component hierarchy it might be fiddly to use the webpack feature in chart builder.

[charlesons]

Loading the react-plotly.js import using React.lazy in ChartPreview.tsx seems to have resolved the issue - more testing required to be sure.

const Plot = React.lazy(() => import("react-plotly.js"));

Source: https://reactjs.org/docs/code-splitting.html

Perhaps there's a more optimal solution using @loadable/component which is in fact already included within the volto project.

https://medium.com/1mgofficial/code-splitting-ssr-lazy-loading-react-components-a-deeper-understanding-part-1-7d714196706

[nosnickid]

Next steps

• [x] Fix flexbox problem
• [x] Which should enable the full create -> save -> edit lifecycle
• [ ] Make the various static texts editable in the volto style
• [x] Use plone file source for CSV

[nosnickid]

The latest changes to this now let you select a CSV file uploaded as a regular 'File' content in plone.

It fetches the CSV from plone and does all the same parsing it does when using the browser file selection.

[nosnickid]

To save the chart in plone, we naively JSON.stringify the state that chart builder has.

This isn't really ideal. For example, once the CSV has been parsed, and dimensions selected, data series are generated from the raw data. We currently store all series in plone, even the ones that aren't selected. And worse, when a client views a page with a chart, all those generated series are included in the block data. There's no need for a client viewing a page with a chart to download all the values that we are currently storing in the block.

We can much smarter about how we serialize the chart builder block; most of what we serialize can be derived from the raw data and the some of the config values.

That task is worth another ticket on its own. For the sake of the March prototype, we can live with the current version.

[nosnickid]

This is now merged into main via #126