Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit parameters to link-to break during animation #347

Open
drlogout opened this issue Aug 5, 2015 · 31 comments
Open

Implicit parameters to link-to break during animation #347

drlogout opened this issue Aug 5, 2015 · 31 comments

Comments

@drlogout
Copy link

drlogout commented Aug 5, 2015

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
Copy link
Author

drlogout commented Aug 5, 2015

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
Copy link

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
Copy link

bugant commented Aug 12, 2015

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
Copy link
Author

drlogout commented Sep 6, 2015

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
Copy link

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:

  • Set up an ember app with routes like:
    • /blogs/:blog_id/posts/:post_id
    • /authors/:author_id
  • In the blog template, link-to 'blog.posts' postId. Note the implicit blog_id param.
  • Go to an author detail page, then move to a post detail page, then click 'back' and you will get the error.

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

@ef4
Copy link
Collaborator

ef4 commented Sep 30, 2015

@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
Copy link

Yes, making the parameter explicit fixes the issue

@drlogout
Copy link
Author

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
Copy link

Eptis commented Oct 7, 2015

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
Copy link

Eptis commented Oct 7, 2015

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

@averydev
Copy link

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
Copy link

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

@ef4
Copy link
Collaborator

ef4 commented Oct 27, 2015

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
Copy link

+1

@lwblackledge
Copy link

+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
Copy link

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
Copy link
Collaborator

ef4 commented Mar 7, 2016

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
Copy link

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
Copy link

@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
Copy link

andyo729 commented Aug 5, 2016

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
Copy link
Collaborator

ef4 commented Aug 5, 2016

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
Copy link

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

@ef4
Copy link
Collaborator

ef4 commented Dec 27, 2016

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
Copy link

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
Copy link

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.

@ef4 ef4 changed the title Cannot read property 'shouldSupercede' of undefined Implicit parameters to link-to break during animation Jan 20, 2017
yohanmishkin added a commit to yohanmishkin/crosscheck that referenced this issue Mar 9, 2017
@scottkidder
Copy link
Contributor

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

@jakeleboeuf
Copy link

same same same :)

jlblcc added a commit to jlblcc/mdEditor that referenced this issue Jul 13, 2017
@viniciussbs
Copy link

Ember 3.3 and I'm still facing this issue.

@scottkidder
Copy link
Contributor

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
Copy link

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

@dnstld
Copy link

dnstld commented Feb 12, 2019

+1

ericbai added a commit to TextUp/textup-frontend that referenced this issue May 28, 2019
…e liquid-fire transitions may result in an error (see ember-animation/liquid-fire#347), pull-to-refresh component ignore mouse or touch move events if dragging hasn't started, upgraded ember-g-recaptcha
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests