Child routes cannot re-use param names of parent routes

[runspired]

The following is either a bug in behavior or a bug in the sense that we are missing an assertion on a key constraint of the router.

Twiddle: https://ember-twiddle.com/84ca6381ddc9ed65dffe9916a102e7f0?openFiles=twiddle.json%2C&route=%2Fparent%2F1%2Fchild%2F1

Setup

  this.route('parent', { path: 'parent/:id' }, function() {
    this.route('child', { path: 'child/:id' });
  });

{{#link-to 'parent.child' '1' '2'}} Go to 'parent/1/child/2' {{/link-to}}

Expected Outcome

• link-to points to the url /parent/1/child/2
• id param given to parent is 1
• id param given to child is 2
• url after transitioning is updated to /parent/1/child/2

Observed Outcome

• [BUG] link-to points to the url /parent/2/child/2
• id param given to parent is 1
• id param given to child is 2
• [BUG] url after transitioning is updated to /parent/1/child/1

*Note that the urls are different in both places, the bug is inconsistent as to which id is accidentally used for both params.

Versions Affected

All versions of Ember seem to be affected.

Debugging I've found this has been either a bug or an un-asserted constraint since at least Ember 2.12.

[rwjblue]

This seems (unfortunately) like designed behavior. The params are held in a single level POJO where the keys are the dynamic segment name. There really isn't anything we can do with that data structure to allow duplicates...

We could assert that you've used the same dynamic segment as a parent, but then I suspect that at least some apps rely on the clobbering here....

[runspired]

I think either

1. we should make the params behave in a more expected way, which would mean changing the storage structure (seems like a good time to surface this now given the RFC work around routing that's been going on cc @chadhietala )

or

2. we should assert this constraint and fall back to it being a deprecation if folks truly are relying on this odd behavior.

I have a hard time believing that folks are (unknowingly) relying on this in apps given that in-app and url based transitions will trigger the model hooks with the correct ID, just serialize to the wrong state for link-to and afterwards put the wrong state into the url. In other words, in many ways "this works" until someone shares or reloads a url that isn't correct, which would be a difficult bug to trace back to this issue.

[krisselden]

This is an issue with route recognizer and one I believe we should fix

[sukima]

What about a warning (in development) when we first construct the route maps that the user choose an ambiguous parameter name? Maybe :id is an ambiguous name. Consider being more specific like :post_id or :item_id

Or a check during route construction that a child uses the same param name as a parent. This should be easily check for at the time of assignment I would think.

[pixelhandler]

yeah @runspired this can troll people using the same name for dynamic segments when defining routes.

Maybe :id is an ambiguous name. Consider being more specific like :post_id or :item_id

I think we've trained ourselves to do the above; but it would be great to un-learn that and just define dynamic segments more liberally, e.g. use the same name if the segment is an :id

[richgt]

@krisselden - I'm looking into a bunch of router issues, and looking at grabbing this one (I'm new to jumping into this stuff, learning a lot). Can you be a little more specific re: your comment above?

This is an issue with route recognizer and one I believe we should fix

Is there a specific direction you were leaning towards what we should fix?