Skip to content

Refs #37102 - detect plugins webpack by looking for remoteEntry.js - #10025

Merged
evgeni merged 1 commit into
theforeman:developfrom
evgeni:detect-webpack
Jan 30, 2024
Merged

Refs #37102 - detect plugins webpack by looking for remoteEntry.js#10025
evgeni merged 1 commit into
theforeman:developfrom
evgeni:detect-webpack

Conversation

@evgeni

@evgeni evgeni commented Jan 30, 2024

Copy link
Copy Markdown
Member

Since we switched to Webpack 5, there are no more manifest.json in the plugins webpack folder, but we need some token to detect that we gotta load webpack stuff.

@evgeni

evgeni commented Jan 30, 2024

Copy link
Copy Markdown
Member Author

For devel setups and Debian this works by checking webpack/index.js, but in RPM we exclude this folder from the installed GEM:
https://github.com/theforeman/foreman-packaging/blob/rpm/develop/gem2rpm/foreman_plugin.spec.erb#L30
(it's correct, we don't need the webpack stuff ON the target system, as we don't compile any assets there, but…)

Comment thread app/registries/foreman/plugin/assets.rb Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arguably, webpack_manifest_path.present? can be dropped now
also, this method hits the disk on every page load, we should cache the result in memory (but scope creep!)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which is exactly what I aimed at in #9834 (comment) but I should have pushed further on this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean "hits the disk" part?

@evgeni

evgeni commented Jan 30, 2024

Copy link
Copy Markdown
Member Author

With this applied I get working Katello prod installs with Webpack 5 \o/

@evgeni
evgeni requested a review from MariaAga January 30, 2024 11:34
Comment thread app/registries/foreman/plugin/assets.rb Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is path && (File.file?(File.join(path, 'webpack', 'index.js')) false? I only see it as true all the time

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we use RPMs as we do not ship the webpack folder at all there, as we do not need to compile anything from it on the system where we deploy.

Comment thread app/registries/foreman/plugin/assets.rb Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should do the - -> _ change for tasks here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Updated

Since we switched to Webpack 5, there are no more manifest.json in the
plugins webpack folder, but we need some token to detect that we gotta
load webpack stuff.
@evgeni

evgeni commented Jan 30, 2024

Copy link
Copy Markdown
Member Author

[test unit]

seems unrelated

@evgeni
evgeni merged commit cf47b3c into theforeman:develop Jan 30, 2024
@evgeni
evgeni deleted the detect-webpack branch January 30, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants