Bug: updating one query param causes the other QP to reset to its default value

[lolmaus]

I found another bug with query params: updating one query param causes the other QP to reset to its default value!

The bug only happens in specific circumstances:

• Given controller (let's call it parent) must have a query param (say, foo).
• There must be some other query param (e. g. bar), either in the same controller or in a child controller.
• The child route's model hook must return a pending promise.
• There must be a loading template adjacent to the child route.
• The parent route must be nested under a grandparent route.
• The grandparent route must have a dynamic segment.

The resulting route structure is:

• grandparent
  • parent
    • child (this can be the implicit index route)
    • loading

Now, if you update foo to a non-default value, then updating bar will cause foo to reset to it's default value!

It's crazy, but removing any of the listed prerequisites prevents the bug from manifesting.

Here's a demo: https://ember-twiddle.com/319583e6a61a449259dc5ea1b6133818

I was able to narrow down the issue to this line:

ember.js/packages/ember-routing/lib/system/router.js

Line 830 in 3141f83

queryParams[qp.scopedPropertyName] = appCache.lookup(cacheKey, qp.prop, qp.defaultValue);

          queryParams[qp.scopedPropertyName] = appCache.lookup(cacheKey, qp.prop, qp.defaultValue);

When all prerequisites are met and you update bar, then pausing execution at that line for the foo QP and running appCache.lookup(cacheKey, qp.prop) would return undefined and, subsequently, cause foo to reset to its default value. If you remove at least one prerequisite, then appCache.lookup(cacheKey, qp.prop) returns foo's current value (expected).

Please help. I'm at a loss, I had to finish my work last week but I'm stuck with this. 😭 I've never even heard about appCache and I have no idea why it fails.

[rwjblue]

Do you have a twiddle or repo that we can look at that demonstrates the issue? Based on the special setup circumstances, it is likely that we won't be able to reproduce on our own and a demo would help push this forward...

[lolmaus]

Yes, here's the demo: https://ember-twiddle.com/319583e6a61a449259dc5ea1b6133818

[lolmaus]

It's crazy how specific the prerequisites are. But they are all legit, and the resulting behavior is totally wrong!

Because of it my date range filter is unusable: you can't select both start and end date. And if you then sort a table (sort order being a QP), then both dates reset and table's data gets redownloaded.

[Serabe]

@lolmaus thank you for the reproduction!

[pixelhandler]

@lolmaus yeah that is crazy

[kevinkucharczyk]

I've run into the same issue recently, really surprised me. I've noticed that it doesn't happen when the query params are scoped to the controller:

  queryParams: {
    foo: { scope: 'controller' },
    bar: { scope: 'controller' }, 
  },

Unfortunately in my case I really need the default model scoping. Would be great to see this fixed!

[pixelhandler]

@Serabe @kevinkucharczyk @lolmaus @rwjblue I see that this is still an issue I updated the ember version, v3.4.1, in the twiddle example to confirm.

[ginomiglio]

Still seems to be a problem on 3.18.1.

Here's a fork of the twiddle with that version (and the image removed):
https://ember-twiddle.com/d4ba8cfc7819d4061498a81130a3bb0e

[rreckonerr]

This is happening because _qpChanged is passed qp object as the third parameter with values === null.

ember.js/packages/@ember/-internals/routing/lib/system/route.ts

Lines 975 to 984 in c5bbf86

	_qpChanged(prop: string, value: unknown, qp: QueryParam) {
	if (!qp) {
	return;
	}
	// Update model-dep cache
	let cache = this._bucketCache;
	let cacheKey = calculateCacheKey(qp.route.fullRouteName, qp.parts, qp.values);
	cache.stash(cacheKey, prop, value);
	}

Which results in cacheKey === "grandparent.parent::grandparent-foo:undefined" (all names are fictional and come from the reproduction example). You can try manually hardcoding this value to "grandparent.parent::grandparent-foo:1" and see that the issue is gone.

The problem is that we're setting qp.values only if transition exists when we're calling the controller's setup method

ember.js/packages/@ember/-internals/routing/lib/system/route.ts

Lines 946 to 953 in c5bbf86

	allParams.forEach((prop: string) => {
	let aQp = queryParams.map[prop];
	aQp.values = params;
	let cacheKey = calculateCacheKey(aQp.route.fullRouteName, aQp.parts, aQp.values);
	let value = cache.lookup(cacheKey, prop, aQp.undecoratedDefaultValue);
	set(controller, prop, value);
	});

and don't provide any default values on qp creation in _qp computed property

ember.js/packages/@ember/-internals/routing/lib/system/route.ts

Lines 2343 to 2358 in c5bbf86

	let qp = {
	undecoratedDefaultValue: get(controller!, propName),
	defaultValue,
	serializedDefaultValue: defaultValueSerialized,
	serializedValue: defaultValueSerialized,
	type,
	urlKey,
	prop: propName,
	scopedPropertyName,
	controllerName,
	route: this,
	parts, // provided later when stashNames is called if 'model' scope
	values: null, // provided later when setup is called. no idea why.
	scope,
	};

@rwjblue @lolmaus Do you have any ideas about what can we do about this? Maybe there's a way to provide default values to qp on the stage of its creation?