Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow HMR in Rancher Components and Harvester #13051

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rak-phillip
Copy link
Member

@rak-phillip rak-phillip commented Jan 10, 2025

Summary

This allows HMR to trigger when changes are made to Rancher Components and Harvester by omitting the excludes argument from getWatcherIgnored(). Watch and WatchOptions work to allow Webpack to watch files and recompile whenever they change - I don't see any benefit in excluding Rancher Components and Harvester from this process by default.

As I understand, the excludes variable exists to control production builds; this change will only affect development.

This also includes a change to clean up the appConfig param in shell/vue.config.

The prefix _ is a convention to indicate that a parameter is expected, but unused within a function. We make use of appConfig in the default export, so we can remove the _ and supply a default value for appConfig if nothing is passed.

Areas or cases that should be tested

  • HMR triggers when running development builds and modifying Rancher Components/Harvester

Areas which could experience regressions

Perhaps, we were intentionally excluding these from HMR for a reason that I'm not aware of.

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@rak-phillip rak-phillip self-assigned this Jan 10, 2025
@rak-phillip rak-phillip added this to the v2.11.0 milestone Jan 10, 2025
@rak-phillip
Copy link
Member Author

rak-phillip commented Jan 10, 2025

@torchiaf I'm requesting a review from you because harvester is included the the default excludes as well1. I'm unsure if there are some historical reasons why we would want to omit Harvester directories from HMR.

Footnotes

  1. https://github.com/rancher/dashboard/blob/05462046a3c3680745ff997c378d7be8686e6ffa/vue.config.js#L5

This allows HMR to trigger when changes are made to Rancher Components and Harvester by omitting the `excludes` argument from `getWatcherIgnored()`. Watch and WatchOptions work to allow Webpack to watch files and recompile whenever they change - I don't see any benefit in excluding Rancher Components and Harvester from this process by default.

As I understand, the `excludes` variable exists to control production builds; this change will only affect development.

Signed-off-by: Phillip Rak <[email protected]>
The prefix `_` is a convention to indicate that a parameter is expected, but unused within a function. We make use of `appConfig` in the default export, so we can remove the `_` and supply a default value for `appConfig` if nothing is passed.

Signed-off-by: Phillip Rak <[email protected]>
@rak-phillip rak-phillip force-pushed the chore/hmr-components-harvester branch from 7042a82 to f8ee344 Compare January 10, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants