Rocket.Chat icon indicating copy to clipboard operation
Rocket.Chat copied to clipboard

fix: Remove auto reload

Open geekgonecrazy opened this issue 2 years ago • 4 comments

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

geekgonecrazy avatar Oct 27 '23 04:10 geekgonecrazy

⚠️ 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

changeset-bot[bot] avatar Oct 27 '23 04:10 changeset-bot[bot]

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

Impacted file tree graph

@@             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.

codecov[bot] avatar Oct 27 '23 04:10 codecov[bot]

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.

debdutdeb avatar Oct 27 '23 15:10 debdutdeb

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

dionisio-bot[bot] avatar Apr 12 '24 19:04 dionisio-bot[bot]

closed by https://github.com/RocketChat/Rocket.Chat/pull/35466

ggazzo avatar Jul 28 '25 14:07 ggazzo