fix: Remove auto reload
Proposed changes (including videos or screenshots)
I know very likely a polarizing change request and probably won't get merged. Probably should even be replaced with something.. but proposing this change to start the conversation.
The way the auto reload right now happens is awful. It refreshes out of absolutely no where. Very frequently interrupting the middle of typing something.
Plus we often end up in loops because the package just doesn't work very well. Especially in any case that a rolling upgrade happens. It ends up contributing to problems by causing everyone to reload even before it should. Then if they end up on instance of the old version again..
https://docs.meteor.com/v2.2/packages/autoupdate.html
The refresh will happen in the browser in two different ways: a soft update, and a hard update. If Meteor identifies that only stylesheets were changed, then it will verify if the user’s browser is capable of reloading CSS on the fly, and a soft update will take place. The soft update will replace the old stylesheet with the new stylesheet without triggering a full page reload.
In cases where a change in a server’s or client’s compiled file happens, the hard update will take place: Meteor will force a complete browser reload using the reload package.
We see this sort of thing in boot loop.
Then:
Among other things, the reload package tries do reload the application preserving some unchanged cached files.
I think the reload package makes matters worse by attempting to preserve files. It's already using hashes for files names.. and the browser is already going to obey cache control.. let the browser figure things out.
Personally I think we'd be better off popping a banner up at the top informing that a new version of the application is available and click here to reload. This puts control in their hands isn't disruptive. Gives time for the rollout to actually finish.
The counter argument might be "what about if the backend api changes?" In that case we could handle by making a refresh required to continue using. It's still better than forcefully refreshing like we are in a dev environment and are auto reloading.
Issue(s)
Fixes #30717
Steps to test or reproduce
Further comments
⚠️ No Changeset found
Latest commit: b449b3c675e34fdd3d156c390cd0937e785b5f3b
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 50.94%. Comparing base (270d16a) to head (b449b3c).
:warning: Report is 3209 commits behind head on develop.
Additional details and impacted files
@@ Coverage Diff @@
## develop #30780 +/- ##
===========================================
- Coverage 51.22% 50.94% -0.28%
===========================================
Files 811 808 -3
Lines 15114 15208 +94
Branches 2758 2827 +69
===========================================
+ Hits 7742 7748 +6
- Misses 6961 7019 +58
- Partials 411 441 +30
| Flag | Coverage Δ | |
|---|---|---|
| e2e | 48.20% <ø> (-0.28%) |
:arrow_down: |
| unit | 63.94% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Just a note, I think this will also disable hot reload on development. There is/should be a way to not include this in final bundles.
Looks like this PR is not ready to merge, because of the following issues:
- This PR has conflicts, please resolve them before merging
- This PR is missing the 'stat: QA assured' label
- This PR is not mergeable
- This PR is missing the required milestone or project
Please fix the issues and try again
If you have any trouble, please check the PR guidelines
closed by https://github.com/RocketChat/Rocket.Chat/pull/35466