Investigate React asset filename mismatch between development builds and app expectations

[Montekkundan]

There appears to be a mismatch between the React UMD filenames copied during development builds and the filenames the app expects at runtime.

• webpack.config.ts copies dev filenames in dev mode and minified filenames otherwise
• The app bundles React as an external (React / ReactDOM), so the runtime script tags must line up exactly

If templates/pages expect react.production.min.js and react-dom.production.min.js during a dev build, the app can fail to boot and render a blank page.

[driusan]

This appear to have been introduced by #9446.

Which is loaded is supposed to be changed by the sandbox flag in the config.xml, but that PR added an "isDev" which conditionally copies to htdocs based on an environment variable that we don't use.

[MaximeBICMTL]

I must say I don't know how the NODE_ENV variable is set exactly (I think it is set by Webpack ?), but it seems like a rather standard approach to me, and seems to work as I expect on my machine:

(loris-python) lorisadmin@mmulder-dev:/var/www/loris$ npm run compile

> loris@1.0.0 compile
> webpack --node-env=development

process.env.NODE_ENV = development
Building all modules
^C
(loris-python) lorisadmin@mmulder-dev:/var/www/loris$ npm run build

> loris@1.0.0 build
> webpack --node-env=production

process.env.NODE_ENV = production
Building all modules
^C

I am not sure if choosing which files to load based on the sandbox flag is a good idea as one may want to test production build on their sandbox. I am also feel like pages/templates expecting a production React/ReactDOM file during development is an anti-pattern. But all of that is just my opinion, feel free to ignore it if you want to. I guess it is easily possible to just re-include both files by changing the patterns in the webpack.config.ts file.

[ridz1208]

I think we all agree on not requiring the sandbox flag... it's actually one of the symptoms of the issue we are having in that it is currently required to be set to 1 when you run make dev and we dont want that.

[driusan]

Before #9446 there was no check for sandbox/NODE_ENV/etc. Both were copied. I think that is the correct behaviour as then changing the sandbox flag will not require a re-build.

[MaximeBICMTL]

Before #9446 there was no check for sandbox/NODE_ENV/etc. Both were copied. I think that is the correct behaviour as then changing the sandbox flag will not require a re-build.

I think this is a working solution but I still kinda disagree with it. IMO the problem is that the templates should not choose which files to import based on the sandbox file, as that flag is a backend variable, whereas the front-end (which I think should include templates) should only be based on the build configuration (NODE_ENV). It also feels like a brute force approach of copying two files while only one is needed.

Another potentially easy and cleaner fix would be to rename the React files during the copy so that both the dev and prod files end up with the same name after the copy (example, before copy: react.development.js / react.production.min.js, after copy: react.js). The templates can then require that file unconditionally and get the correct file no matter which one was compiled.

[driusan]

There aren't "backend" and "frontend" config variables in LORIS, only config variables. I don't know what you're referring to. NODE_ENV is not referenced anywhere, in any code or documentation within LORIS, and was only introduced in that PR. It is not a valid LORIS configuration.

I don't know if renaming it when copying would work or not (I know the React bundles work differently but I don't know how they determine which behaviour to use), but it would also result in the wrong behaviour if the config value is changed so I still think they should both be copied.

[MaximeBICMTL]

Regarding NODE_ENV, it is not set manually, it is a standard variable used to detect development and production builds in Webpack, see the following Webpack documentation:
https://webpack.js.org/configuration/mode/

Regarding "backend" and "frontend" configuration, it is more a general software architecture point than a specific LORIS feature. I am deeply convinced that the front-end and back-end of a modern web application should be decoupled. The build mode of the Javascript (dev / prod) belongs to the front-end, whereas in LORIS, the sandbox setting belong to the back-end. It does not make sense to me that the client requires different JavaScript scripts based on the sandbox configuration.