ember-animation / liquid-fire

Animations & transitions for ambitious Ember applications.
Other
1.15k stars 199 forks source link

Implicit parameters to link-to break during animation #347

Open drlogout opened 9 years ago

drlogout commented 9 years ago

I have one {{liquid-outlet}} in my application.hbs

Here's my routing map:

  this.route('info');
  this.route('kontakt', function () {
    this.route('partner-links');
    this.route('impressum');
  });
  this.route("not-found", {path: '404'});

  this.route('index', {path: '/'}, function () {
    this.route('category', {path: '/:category'}, function () {
      this.route('subcategory', {path: ':subcategory'}, function () {
        this.route('post', {path: ':post'}, function () {
        });
      });
    });
  }); 

If I go from the route "index.category.subcategory.post.index" to "info" or "kontakt" I get: "Uncaught TypeError: Cannot read property 'shouldSupercede' of undefined".

This is happening after the transition to "info" or "kontakt" is completed. The transition got stuck, both liquid-child container (the new and the old) are visible.

From all the other routes the transition to "info" & "kontakt" is working.

I'm not sure if this is a problem with liquid-fire or with my routing but this only happens if I use {{liquid-outlet}}. With the normal {{outlet}} it works as expected.

Cheers drlogout

EDIT: ember 1.13.6 ember-cli: 1.13.7 liquid-fire: 0.21.0

drlogout commented 9 years ago

After filing the issue I discovered that

this.route('category', {path: '/:category'}

should be

this.route('category', {path: ':category'}

But changing this doesn't solve the problem.

My new routing map:

this.route('index', {path: '/'}, function () {
    this.route('category', {path: ':category'}, function () {
      this.route('subcategory', {path: ':subcategory'}, function () {
        this.route('post', {path: ':post'}, function () {
        });
      });
    });
  }); 
bugduino commented 9 years ago

We have the same problem in our app, no links error without liquid-fire and random 'Cannot call shoudSupercede of undefined with liquid-fire. Do you find any solution?

( The transitions are not blocked anyway, we get the expected behaviour in the app but lot of errors in console)

ember 1.11.3 ember-cli: 1.13.1 liquid-fire: 0.19.4

Thank you

bugant commented 9 years ago

We are experiencing the same issue. Our route file is OK. From some investigations, it seems that when transitioning to the new route, something tells Ember to compute route from the previous one, but not all data are still in place (read dynamic segments) to being able to calculate the path.

The transitions works fine, but I find errors logged in the console.

Here is the backtrace:

TypeError: Cannot read property 'shouldSupercede' of undefined
    at Object.__exports__.default.subclass.applyToHandlers (http://localhost:4200/assets/vendor.js:56357:53)
    at Object.__exports__.default.subclass.applyToState (http://localhost:4200/assets/vendor.js:56296:21)
    at Object.Router.applyIntent (http://localhost:4200/assets/vendor.js:55731:23)
    at calculatePostTransitionState (http://localhost:4200/assets/vendor.js:34739:26)
    at EmberObject.default.extend._hydrateUnsuppliedQueryParams (http://localhost:4200/assets/vendor.js:34500:19)
    at EmberObject.default.extend._prepareQueryParams (http://localhost:4200/assets/vendor.js:34439:12)
    at computeLinkViewHref (http://localhost:4200/assets/vendor.js:30099:14)
    at Descriptor.ComputedPropertyPrototype.get (http://localhost:4200/assets/vendor.js:20938:26)
    at Object.get (http://localhost:4200/assets/vendor.js:26198:19)
    at Object.merge.default.valueFn (http://localhost:4200/assets/vendor.js:48324:29) "TypeError: Cannot read property 'shouldSupercede' of undefined
    at Object.__exports__.default.subclass.applyToHandlers (http://localhost:4200/assets/vendor.js:56357:53)
    at Object.__exports__.default.subclass.applyToState (http://localhost:4200/assets/vendor.js:56296:21)
    at Object.Router.applyIntent (http://localhost:4200/assets/vendor.js:55731:23)
    at calculatePostTransitionState (http://localhost:4200/assets/vendor.js:34739:26)
    at EmberObject.default.extend._hydrateUnsuppliedQueryParams (http://localhost:4200/assets/vendor.js:34500:19)
    at EmberObject.default.extend._prepareQueryParams (http://localhost:4200/assets/vendor.js:34439:12)
    at computeLinkViewHref (http://localhost:4200/assets/vendor.js:30099:14)
    at Descriptor.ComputedPropertyPrototype.get (http://localhost:4200/assets/vendor.js:20938:26)
    at Object.get (http://localhost:4200/assets/vendor.js:26198:19)
    at Object.merge.default.valueFn (http://localhost:4200/assets/vendor.js:48324:29)"
drlogout commented 9 years ago

I figured it out. The reason was an unnecessary route name in a link-to helper which actually needed only query-params. Because the error only appeared with liquid-outlet I was on the wrong track.

richmolj commented 9 years ago

I believe I know what's causing this, generally. If you have a link-to with an implicit url param, liquid fire will break when moving to another route that does not contain the same implicit param.

To reproduce:

I would imagine this is liquid fire trying to keep the old template around for transitions, but it's not renderable.

ef4 commented 9 years ago

@richmolj That sounds very plausible. If your diagnosis is correct, it should be possible to workaround by making the parameter explicit instead: {{link-to 'blog.posts' model.id postId}}. Does that work?

richmolj commented 9 years ago

Yes, making the parameter explicit fixes the issue

drlogout commented 9 years ago

It worked for me after I removed the route name in those links where only the query params changed (within the same route):

-        {{#link-to 'index.category.subcategory.post' (query-params bild=(inc index)) }}
+        {{#link-to (query-params bild=(inc index)) }}
Eptis commented 9 years ago

I'm still experiencing this issue with liquid-outlet without any link to helpers in my template (on my new route). Any suggestions on how to debug this further? Changing to outlet solves it

Eptis commented 9 years ago

Ah nevermind, I found the error, was a link-to helper in my old route

averydev commented 9 years ago

I'm having what seems to be a similar issue. When I attempt to return to the application route or index route I get an Cannot read property 'shouldSupercede' of undefined error.

It's a tricky error and I'm not quite sure where to look to debug...

ember: 1.13.8 ember-cli: 1.13.8 liquid-fire 0. 21.2

svvitale commented 8 years ago

Holy hell, THANK YOU for figuring this out and documenting it! I NEVER would have gotten there on my own...

ef4 commented 8 years ago

Yes, this one is painful. It's not something we can easily fix (or even warn about) within liquid-fire's own code, and even if we were able to cut off all streams flowing into the old view (a nice feature I want to work on anyway), I'm not sure it would help with this case. I will need to see about making a change within Ember.

dangreenisrael commented 8 years ago

+1

lwblackledge commented 8 years ago

+1

We ran into this issue when updating to Ember 1.13. The route in question has two dynamic segments as part of the resource definition, like this:

this.resource('blog', { path: ':blog_id' }, function() {
  this.resource('blog.posts', { path: 'posts' }, function() {
    this.resource('posts.comments', { path: ':comment-author_id/:comment-category_id/comments' }, function() {
      this.resource('posts.comment', { path: ':comment_id' }, function() {

The link-to that fires this error is on the 'blog' index.hbs:

{{#link-to "blog" blog class="icon icon-blog"}}{{/link-to}}

This works fine if fired from 'posts.comment', but throws the error if fired from 'posts.comments'. If I switch the template to render {{outlet}} instead of {{liquid-outlet}} we do not get this error.

As @richmolj said, I think it is liquid-fire trying to keep the template for transitions by actually re-rendering it, but failing (incidentally the re-rendering seems a bit redundant to me since the HTML is already there for the user to see?).

If it is useful, the error fires from ''router/transition-intent/named-transition-intent'.applyToHandlers() as this line if (i >= invalidateIndex || newHandlerInfo.shouldSupercede(oldHandlerInfo)) { because newHandlerInfo is null.

ampatspell commented 8 years ago

I have slightly different case with the same error and stack trace.

Routes are:

this.route('home', { path: '/' }, function() {
  this.route('guest', function() {
  });
  this.route('staff', function() {
    this.route('hotel', { path: '/:hotel_id' }, function() {
    });
  });
});
this.route('session', function() {
});

And I'm getting the same error when I try to transition from home.staff.hotel.index to session.index.

But home.guest to session.index works fine. Same deal works only when there is no dynamic segments.

transitions.js has rule for home to and back from session:

this.transition(
  this.fromRoute('home'),
  this.toRoute('session'),
  ...ios
);

Which is used for both home.staff.. and home.guest transitions.

Uncaught TypeError: Cannot read property 'shouldSupercede' of undefined
exports.default._routerUtils.subclass.applyToHandlers
exports.default._routerUtils.subclass.applyToState
Router.applyIntent
calculatePostTransitionState
_emberRuntimeSystemObject.default.extend._hydrateUnsuppliedQueryParams
_emberRuntimeSystemObject.default.extend._prepareQueryParams
exports.default._emberRuntimeSystemService.default.extend.normalizeQueryParams
exports.default._emberRuntimeSystemService.default.extend.generateURL
computeLinkToComponentHref
...

Ember: 2.3.0 Liquid-Fire: 0.23.0

Any pointers would be highly appreciated.

ef4 commented 8 years ago

It sounds like the same bug as above, which means you can work around it by setting all the parameters explicitly in your link-tos.

Look at all your link-to's in home.staff.hotel and home.staff.hotel.index. Probably one of them is expecting to use the current id implicitly, which doesn't work during animations since the link-to will need to be rendered under the "wrong" route while it's animating.

ampatspell commented 8 years ago

Turns out I simply had a typo in one of nested-nested-nested component link-to's model prop (link-to is almost never visible so w/o this error thrown I haven't seen it live). Model for dynamic segment was missing, so yeah, error makes complete sense.

Thanks for the help! Cheers

marinatedpork commented 8 years ago

@ef4 What exactly do you mean by explicitly setting all my parameters in my link-to helpers? What is implicit vs explicit?

I am experiencing the same issue as above, only happens with a liquid-outlet, but to my understanding of link-to, all of them are being set explicitly:

    {{#link-to "portfolio.student-domain" model.portfolio.id model.id}}
      <button class="btn btn-primary">
        copy
      </button>
    {{/link-to}}

    {{#link-to "portfolios" (query-params page="A" sort="asc")}}
        oink
    {{/link-to}}

Thank you for your time

edit: I ended up removing most of all my links and now things are flowing.

andyo729 commented 8 years ago

I have resolved this issue by explicit link-to.

It would be nice to not have to remember to use explicit link-to. Does any one have any method to help call out implicit link-to? If not, I think this issue is definitely worth continuing to look into.

Not sure if this helps the discussion, but I noticed that I don't have to even click the link-to in order for this to happen. If it's on the route, it affects liquid-outlet.

ef4 commented 8 years ago

I want to refactor link-to within ember itself to make it faster and to solve this class of problem.

Right now each link-to looks at a router service to decide whether it should be active and how to fill in implicit parameters. But it could instead look at the locally scoped outletState, the same way outlets do.

kylemellander commented 7 years ago

Is there any update on this? I am still getting this regularly when transitioning beyond a liquid-outlet

ef4 commented 7 years ago

The current solution is to make sure your link-tos are not relying on implicit parameters. A more complete fix will depend on people working to implement Ember RFC 95.

kylemellander commented 7 years ago

I will look into helping on that. Just to note, this is happening with our wildcard route, which I cannot seem to get around this since its not a true link-to.

Duder-onomy commented 7 years ago

I hate to harp on this when it seems like people are already working on it.

Can someone demonstrate how to do what @ef4 mentions 4 posts up?

link-to looks at a router service to decide whether it should be active and how to fill in implicit parameters. But it could instead look at the locally scoped outletState, the same way outlets do.

We could all individually implement some kind of {{liquid-link-to}} that we all use until the RFC 95 is merged.

scottkidder commented 7 years ago

+1 for running into this and being somewhat confused for a moment.

jakeleboeuf commented 7 years ago

same same same :)

viniciussbs commented 6 years ago

Ember 3.3 and I'm still facing this issue.

scottkidder commented 6 years ago

Yep, I now consider it a matter of course to go through the work of keeping explicit parameters on all my {{link-to}}'s. It will be nice when I'm able to wipe all these out someday.

viniciussbs commented 5 years ago

@ef4 Any news after the merge of the router service? Or is this waiting for router helpers and modifiers?

dnstld commented 5 years ago

+1