emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.45k stars 4.21k forks source link

[Bug] Error when transitioning to a route with QPs #19483

Open wagenet opened 3 years ago

wagenet commented 3 years ago

🐞 Describe the Bug

I am transitioning to a route that has a :name in the url and also QPs. This transition can be slow. If, while the transition is still taking place, I try to transition to the same route with different QPs, I get the following error.

Error: You must provide param `name` to `generate`.
    at getParam (route-recognizer.js:137)
    at Array.generate.<computed> (route-recognizer.js:183)
    at RouteRecognizer.generate$1 [as generate] (route-recognizer.js:553)
    at PrivateRouter._updateURL (router_js.js:1651)
    at router_js.js:1250
    at invokeCallback (rsvp.js:485)
    at rsvp.js:546
    at rsvp.js:14
    at invokeWithOnError (backburner.js:272)
    at Queue.flush (backburner.js:182)

🔬 Minimal Reproduction

TODO

😕 Actual Behavior

My theory is that the router is trying to update the QPs on the current route, not the future one. In the above example, if I pause where the error is thrown, I can see that params are the params of the current route, not the future one.

🤔 Expected Behavior

This doesn't raise an error.

🌍 Environment

wagenet commented 3 years ago

It looks like the second transition ends up marked as a queryParamsOnly transition which doesn't work correctly in this context.

sly7-7 commented 3 years ago

It would be nice to have your exact scenario here. I will take a look at it this evening.

sly7-7 commented 3 years ago

@wagenet I tried to write a test to reproduce you use case, but it fails with an other error.

async ['@test repro 19483'](assert) {
      assert.expect(1);
      this.application.LOG_TRANSITIONS = true;
      this.application.LOG_TRANSITIONS_INTERNAL = true;
      this.addTemplate('application', '{{outlet}}');
      this.addTemplate('posts', '{{outlet}}');

      this.router.map(function () {
        this.route('posts');
      });

      //this.add('route:application_loading', Route.extend());

      this.add(
        'route:posts',
        Route.extend({
          model(params, transition) {
            return new Promise((resolve) => {
              if (transition.to.queryParams.a === '42') {
                later(this, resolve, 1000);
              } else {
                assert.ok(true);
                resolve();
              }
            });
          },
        })
      );

      this.visit('/posts?a=42');
      await this.visit('/posts?a=45');
    }
Source: | TypeError: Cannot read property 'name' of undefined     at Class._queryParamsFor (http://localhost:4200/tests/ember.js:22157:59)     at Class.finalizeQueryParamChange (http://localhost:4200/tests/ember.js:21106:29)     at Class.triggerEvent (http://localhost:4200/tests/ember.js:22616:27)     at PrivateRouter.triggerEvent (http://localhost:4200/tests/ember.js:21466:43)     at PrivateRouter.finalizeQueryParamChange (http://localhost:4200/tests/ember.js:66003:12)     at PrivateRouter.queryParamsTransition (http://localhost:4200/tests/ember.js:65498:37)     at PrivateRouter.getTransitionByIntent (http://localhost:4200/tests/ember.js:65570:37)     at PrivateRouter.transitionByIntent (http://localhost:4200/tests/ember.js:65519:21)     at PrivateRouter.doTransition (http://localhost:4200/tests/ember.js:65655:19)     at PrivateRouter.handleURL (http://localhost:4200/tests/ember.js:66111:19)
-- | --

So I don't know if this is the exact reproduction, but anyway... this is bad :(

wagenet commented 3 years ago

This definitely seems similar to what happened to me. We can see if resolving this issue resolved mine as well.

lvegerano commented 3 years ago

Look for bugs labeled QueryParams, this issue has been lurking circa 2015.

chancancode commented 7 months ago

I noticed the workaround @wagenet added in our codebase and decided to revisit and check whether it is still necessary, and the answer is yes, it is still an issue as of Ember 5.4 :)