Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Consider how govuk-react-jsx is packaged up as a UMD. Switch to ESM? Both? #148

Open
andymantell opened this issue May 19, 2022 · 3 comments
Labels
enhancement New feature or request tech-debt

Comments

@andymantell
Copy link
Collaborator

Consider how govuk-react-jsx is packaged up as a UMD. Switch to ESM? Both?

Need to do some research and look at what the current best way is to do this.

See GSS-Cogs/dd-cms#49 where they experienced some issues with the current packaging.

@nosnickid
Copy link

nosnickid commented May 19, 2022

Some more details about the problem we had.

We used surevine/govuk-react-jsx to build some components for plone/volto, a React frontend for CMS

plone/volto in turn uses jaredpalmer/razzle to run a dev server, to build prod packages, etc. razzle uses webpack to provide that functionality. One of the main reasons for using razzle is its reliable server side rendering support, both in dev and prod. It does this by running two (almost) identical webpack configs in a webpack instance. I'll call them the 'SSR' and 'client side' below...

One of our components uses govuk-react-jsx.Header. Header imports govuk-frontend.govuk-logotype-crown.png, in the distributed assets via a require() (govuk-react-jsx@6.2.1:govuk/components/header/index.js:22):

var _govukLogotypeCrown = _interopRequireDefault(require("govuk-frontend/govuk/assets/images/govuk-logotype-crown.png"));

In the SSR, webpack was treating govuk-react-jsx as an "external" dependency, which means it doesn't do any interception of require() statements. This was causing node to try to import the PNG as a JS file directly:

.../volto/node_modules/govuk-frontend/govuk/assets/images/govuk-logotype-crown.png:1
�PNG

SyntaxError: Invalid or unexpected token
    at wrapSafe (internal/modules/cjs/loader.js:1001:16)

I didn't dig in deep enough to figure out why the client side config did intercept the require and use the image-loader, but it must have been doing so.

In the end our workaround was to have SSR not treat govuk-react-jsx as an 'external' dep, the upshot being webpack rewrote the require() for a PNG.

I think ESM vs UMD might be a distraction here. In writing this up I'm still not satisfied I fully understand why the externals hack fixed this for us, but it was a case of moving on with the rest of the project.

@andymantell
Copy link
Collaborator Author

Ah ok that's interesting. And gnarly sounding 😬 Similar discussions went on in #107 - I suspect if you did look at both the webpack configs you would find that one was configured with a loader to handle the image imports and the other wasn't.

Totally appreciate you've got a solution now, and you've largely moved on! But if you do think anything needs changing at my end please let me know and we can thrash it out. Although as you no doubt appreciate it's quite hard to support all of the exotic ways people consume this package! Fun, but mind bending...

@nosnickid
Copy link

Yeah it was a bit hairy - actually I think I said mind bending a few times too :)

Pretty sure I found that ticket back in Feb, along with a few ones about similar NextJS problems.

There is an image loader in the SSR config; the trick was convincing webpack to process require()s. It had the same problem in trying to directly require() CSS files in the node side.

I spent some time examining other React component libraries. I didn't find one that one had images, but where they had CSS(from css/less/sass), in the distributed assets they had the import CSS statements removed, used the mini-css-extract-plugin to write a CSS file, and expected the consumer to import the CSS directly.

That's fine for CSS, but a pain for inline <image>s!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request tech-debt
Projects
None yet
Development

No branches or pull requests

2 participants