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

Clicking a link build with the {{link}} helper, the transition is not executed in Edge <= 18 #459

Closed
dcyriller opened this issue Oct 14, 2020 · 3 comments

Comments

@dcyriller
Copy link

dcyriller commented Oct 14, 2020

Hi there 👋,

Thanks for this addon, that we use. :)
I think I found a bug. AFAICT the issue would be in ember-source. But I'm opening the issue here for tracking purpose.

A bug

Clicking a link build with the {{link}} helper, the transition is not executed in Edge <= 18.

Instead, I get the following error in the console:
Unable to get property 'router' of undefined or null reference
on the following line:

return this._linkManager.router.transitionTo(...this._routeArgs);

A possible cause

The bug only happens in Edge <= 18. I wonder if the code:

ember-link/addon/link.ts

Lines 276 to 283 in 8f0fab3

@action
transitionTo(event?: Event | unknown): Transition {
// Intentionally putting this *before* the assertion to prevent navigating
// away in case of a failed assertion.
this.preventDefault(event);
return super.transitionTo();
}
would not be a reproduction for this issue in the framework: emberjs/ember.js#18955

@buschtoens
Copy link
Owner

Great detective work! 🕵️

I think you found the correct root cause. You could try replacing super with something like the following, to test your theory:

return Link.prototype.transitionTo.call(this);

@dcyriller
Copy link
Author

dcyriller commented Oct 14, 2020

Great detective work!

😆 it was indeed detective work.

Thanks for the pointer... It did not work as is but gave me the idea to inline Link's transitionTo into UILink... And the bug is indeed fixed when removing super. I'm going to report the details of my investigation in the cross-linked issue.

What would be the best way forward regarding the bug in ember-link? Do you prefer to wait for the root cause to be fixed? Or do you wish to inline Link's transitionTo into UILink?

@buschtoens
Copy link
Owner

buschtoens commented Oct 14, 2020

Ah yes, that didn't work because @action actually replaces the original prototype value with a getter: https://github.com/emberjs/ember.js/blob/master/packages/%40ember/object/index.js#L120-L152

Because the @action decorator stashes the real function away into a private BINDINGS_MAP, we also cannot conveniently access it again, to apply a monkey-patch fix. :(

As Edge <= 18 isn't really in the support matrix of this addon, I'd ideally like to wait for an upstream fix, but I'd also like to unblock you.

Can you maybe answer the questions from emberjs/ember.js#18955 (comment) ?

Hmm, this is pretty odd. Some questions:

  • What are you config/targets.js?
  • Is this the same in dev vs prod builds?
  • Can you share the transpiled output of both files?
GitHub
Ember.js - A JavaScript framework for creating ambitious web applications - emberjs/ember.js

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

2 participants