[Bug] Route.refresh() persists old queryParams values after programmatic updates

[elwayman02]

🐞 Describe the Bug

Calling refresh() on a route after updating queryParams results in the old queryParams values persisting and the new values being lost. This behavior is necessary to work for the scenario where you want a query param to have refreshModel: false and only execute the model hooks when refresh() is called programmatically.

🔬 Minimal Reproduction

Describe steps to reproduce. If possible, please, share a link with a minimal reproduction.

1. Add a query param to a route
2. Programmatically change the value of that query param on the controller
3. Immediately call refresh() on the route

https://ember-twiddle.com/c43e0da2ee0078f148ad492e6c43c44e?openFiles=routes.my-route%5C.js%2C

😕 Actual Behavior

As you can see from the twiddle above, there are three scenarios we care about here:

1. The query param is defined with refreshModel: true and the value is updated via controller. The model hook executes and the value is correct.
2. Same as scenario 1, but calling refresh() immediately after updated the value. The model hook executes and the value is incorrect.
3. Same as scenario 2, but the query param is defined with refreshModel: false. The model hook executes and the value is incorrect.

🤔 Expected Behavior

In all three scenarios, the same behavior should result - the query param should be updated and the new value should be available when the model hook executes.

🌍 Environment

• Ember: - We first started seeing this behavior change introduced when upgrading from Ember 3.12 to 3.16. It worked as expected in 3.12 but is now broken in 3.16+
• Node.js/npm: - 12.13
• OS: - macOS 10.15.7
• Browser: - Chrome

➕ Additional Context

Add any other context about the problem here.

[rwjblue]

You can work around this issue by using transitionTo instead of using the controller property to set the new value.

I put together a small repo to make testing various things a bit easier (https://github.com/rwjblue/__emberjs19190).

• https://github.com/rwjblue/__emberjs19190/commit/b86e08d537dedfcc547d6e7fbb7562182bfa273c - The reproduces the issue from the twiddle.
• https://github.com/rwjblue/__emberjs19190/commit/f5163fde8b15c6aa932f5a1c53243bbed1128181 - Shows usage of .transitionTo instead of controller property assignment (aka two way binding)

The last commit there in that repo has the behavior that is desired.

[sly7-7]

I'm playing a bit with this example (againt router.js and ember.js both on their master branch) and have a couple of observations (which won't resolve the issue...).

1. First (working) scenario: There is something weird to me though, because the { refreshModel: true } starts two 'refresh transition':

2. The exposed scenario does not make sense to me here, because there are both { refreshModel: true }, and manual call to refresh(), which sounds redundant (but either, maybe this should work though).

3. This is a kind of async issue for me. Actually, if you wrap the refresh call inside a run.next(), that works (but I'm almost sure this is not recommended). From what I see, when chaining controller.set property and refresh, it's like the 'refresh transition' triggered by refresh() is masking/overriding/avoidin the qp observers to be fired (because on the same run loop I guess).

Question now: Do we want to make those scenarios working anyway, or just close this issue since there is a 'workaround' (using transitionTo with QP only), which IMO should be the preffered way.

[Techn1x]

Hit this one today too. Still around in Ember 4.4. Workaround as described above seems to still work (either transitionTo or replaceWith)