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

Make transitionTo work with url as an argument #39

Merged
merged 1 commit into from
Jul 12, 2021

Conversation

SergeAstapov
Copy link
Contributor

@villander this recreates #32 since as you mentioned ember-engines/ember-engines#587 (comment) does not happen.

@villander villander self-requested a review July 9, 2021 20:38
@villander
Copy link
Owner

@SergeAstapov are you sure that it will work? Because we have a native error here from ember core codebase - https://github.com/emberjs/ember.js/blob/cf7c1bdebcd6af2e9d269ef3fd7946a29455c5df/packages/%40ember/-internals/routing/lib/utils.ts#L214

@SergeAstapov
Copy link
Contributor Author

SergeAstapov commented Jul 10, 2021

@villander it worked for me and I believe it's not an issue here because we do

    if (resemblesURL(routeName)) {
      return get(this, 'externalRouter').transitionTo(routeName);
    }

so transitionTo is called on the externalRouter where we should have owner.routable = false because owner for the externalRouter is the host app, according to

return this.rootApplication.lookup('service:router');

Anyways, let me double check to make sure it works as intended.

@villander
Copy link
Owner

That makes sense @SergeAstapov

Thank you so much for push it forward ; )

@villander villander merged commit 433a89f into villander:master Jul 12, 2021
@villander villander added the bug Something isn't working label Jul 12, 2021
@SergeAstapov SergeAstapov deleted the transitionTo-with-url branch July 12, 2021 17:55
@SergeAstapov
Copy link
Contributor Author

@villander do you think you could release a patch version?

@villander
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants