Fixes #37252 - prevent duplicate foremanReact in plugins

[MariaAga]

• Instead of each plugin creating a copy of foremanReact things, to use external to tell it "its already loaded".
• to actually give plugins access to foremanReact things create an entry file that exports all (all_react_app_exports.js) things. (we cant create an entry for each file as that crashes and not effective).
• to make sure everything in the webpack/assets/javascripts/react_app is actually exported, generate all_react_app_exports each webpack build with a script (webpack/assets/javascripts/exportAll.js)
• the script skips tests as they are not imported and tests mock files if needed
• change webpack/assets/javascripts/react_app/mockRequests.js to run only when called otherwise axios only uses the mock, another option is to exclude it from the import script.
• Leaving BundleAnalyzerPlugin in while this is a draft pr
  katello sizes before & after this change:
  
  

I tested some pages and they look like they work but will do more checks

[jeremylenz]

Checked katello pages (React, Angular, erb) and seems to work fine.

A rebase should fix the 3 failing katello tests.

Anything in particular that should be tested?

[MariaAga]

Anything in particular that should be tested?

That this solution makes sense :)
and that it won't damage the build/packaging process (@theforeman/packaging)

[MariaAga]

This is ready for review/ merge on my side.
Test failures are unrelated.
Pinging @theforeman/packaging again

[evgeni]

do we need to store the file in the repo? or is it sufficient when webpack builds it for us when we run webpack:compile?

[MariaAga]

We dont have to but I thought it might be helpful to have in the repo, to keep track of things/find issues

[evgeni]

@ehelms @ekohl opinions?

I'd prefer not to have it in the repo, but would love to hear other thoughts.

[ekohl]

Like @evgeni I'd prefer to not have it in the repo. I think it will only be a source of noise or merge conflicts.

What we should do is archive it as part of CI so it can be inspected, just like we do with package-lock.json / Gemfile.lock. I think that would provide sufficient insight.

[MariaAga]

Added to gitignore, and I'll create a pr for the actions

[evgeni]

[test unit]
[test katello]
[test integration]

[evgeni]

So I've built a Katello based on this, and the bundle size did shrink!

Before:

• 107.js (5.73 MiB)
• 732.js (5.99 MiB)

After:

• 107.js (2.29 MiB)
• 732.js (2.54 MiB)

But now Katello pages are broken with:

Uncaught (in promise) TypeError: TheForeman.reactExports is undefined

Is there anything I'm missing that would be needed on the Katello side to consume that?

[MariaAga]

Nope, should work as is, how exactly did you build?

[evgeni]

I took the packit build from this PR and threw it into the build chain of katello.rpm.
So "as usual" you could say.

[evgeni]

When built, /usr/share/foreman/public/webpack/reactExports.js does contain var TheForeman, but not reactExports.

[MariaAga]

This is a prod only issue, fixed by changing

library: {
      name: ['TheForeman', '[id]'],

From id to name so that it will always be named "reactExports" and not some id

[evgeni]

Updating to the packit build with name fixes it!

[MariaAga]

So we fixed #10042, but plugins are still copying foremans old code if they are not rebuilt (https://community.theforeman.org/t/problem-foreman-3-10-after-upgrade/37459)
Should we / can we CP this into 3.10?

[evgeni]

RPM builds fail with:

[webpack-cli] Failed to load '/builddir/build/BUILD/foreman-3.11.0/config/webpack.config.js' config
[webpack-cli] Error: Cannot find module 'webpack-bundle-analyzer'

is bundle-analyzer something we need in production builds? today we exclude that from packaging

[MariaAga]

is bundle-analyzer something we need in production builds? today we exclude that from packaging

Sorry, that was a leftover

[ekohl]

Should we / can we CP this into 3.10?

I don't think we should. It feels very risky and likely to break things. That's why we have a regular release cycle where release often enough that it shouldn't matter that much if you miss a merge window.

[evgeni]

Module not found: Error: Can't resolve '@theforeman/test' in '/builddir/build/BUILD/foreman-3.11.0/webpack/assets/javascripts/react_app'

🤔 why does it try to load @theforeman/test

[evgeni]

heureka!
@MariaAga does 605f110 make sense (and: can it be written any nicer?)

TL;DR:
webpack/assets/javascripts/react_app/mockRequests.js imports @theforeman/test but it was not on your "do not include" list

[MariaAga]

@MariaAga does 605f110 make sense (and: can it be written any nicer?)

Yes, I had it originally like that but thought I could work around it by editing mockRequests

[evgeni]

meh, somehow this got reverted back to name: ['TheForeman', '[id]'], instead if name: ['TheForeman', '[name]'],, fixing

[evgeni]

So I have no idea what the changes to mockRequests.js do (or should do), but the webpack bundle does shrink and this PR works fine (with my two changes, but they aren't really mine anyway).

I'd be OK merging this as is.

@ehelms @ekohl any objections?

[jeremylenz]

[test katello]

[evgeni]

[test katello]
[test integration]

[ekohl]

Happy to see this happen.