Skip to content

Introduce external redirects - #220

Closed
kvas-damian wants to merge 4 commits into
emberjs:masterfrom
Wikia:rfc-external-redirects
Closed

kvas-damian wants to merge 4 commits into
emberjs:masterfrom
Wikia:rfc-external-redirects

Conversation

@kvas-damian

Copy link
Copy Markdown

@joukevandermaas

joukevandermaas commented Apr 12, 2017 •

Copy link
Copy Markdown

I wonder what is the reason this has to live on the route instead of as a separate package? Example:

import { externalTransitionTo } from 'ember-external-transition';

externalTransitionTo('someplace');

@kvas-damian

Copy link
Copy Markdown
Author

@joukevandermaas Currently routes are responsible for redirecting to another route. Extracting externalTransitionTo to a separate package makes it less intuitive to find.

Can you share your reasons why we shouldn't put it in the Route?

@joukevandermaas

Copy link
Copy Markdown

@kvas-damian I don't necessarily have strong feelings one way or the other, but it is an alternative. Since the entirety of FastBoot is a package (the RFC mentions FastBoot as one of the main reasons for the necessity of this new API), it might make sense to keep this separate also.

@kvas-damian

Copy link
Copy Markdown
Author

@joukevandermaas In our use-case FastBoot isn't the only consumer of this API. Our application requires this method on frontend as well.

I'm also ok with implementing it as separate package when we agree that it's better way.

@courajs

courajs commented Apr 14, 2017

Copy link
Copy Markdown

I don't really think reducing your six-line snippet down to one line justifies adding new core API, and making it harder to teach how routing works in Ember. This is an issue that pretty much only will come up when doing FastBoot/server rendering, as otherwise the browser API works perfectly well.
I'm sure some people would appreciate if a wrapper around the two approaches was published as an addon.

@cibernox

cibernox commented Apr 19, 2017 •

Copy link
Copy Markdown
Contributor

I also see this as a 5 lines utility rather than something to be added to the code. In the frontend, window.location = 'https://new-location.com' is enough.

Only in fastboot it's a bit trickier, so could be a method in the fastboot service tho.

if (this.get('fastboot.isFastBoot')) {
  this.get('fastboot').redirectTo('https://google.com', { status: 302 });
} else {
  // similar behavior as an HTTP redirect
  window.location.replace("http://stackoverflow.com");

  // similar behavior as clicking on a link
  window.location.href = "http://stackoverflow.com";
}

@courajs

courajs commented Apr 28, 2017

Copy link
Copy Markdown

This also conflicts to some extent with the external transitions of engines: http://ember-engines.com/guide/linking-and-external-links

@les2

les2 commented Jun 10, 2017

Copy link
Copy Markdown
Contributor

at work we created a service called navigation for routing to external applications.

we define the external routes in config and use an api similar to transitionTo.

before engines was a thing, we developed a{{link-to-external helper for usage in templates. haven't changed the name yet ...

this architecture made it very easy to test by mocking out the navigation service, preventing tests from navigating the test runner to external sites, verify in tests that external links are functioning properly. it also neatly defined the external routes in one place.

i wish ember came with all that. :)

the point of all that was to say that 1) i think this api proposal would be useful, 2) i think the use cases could be taken a little further, e g., allow configuring a "map' for external urls when possible, 3) should be possible to implement in an add-on without modifying ember core.

the alternative of having if statements all over the place that bifurcate code base on fastboot availability is in my view just horrible. :sadpanda:

@locks

locks commented Jun 10, 2017

Copy link
Copy Markdown
Contributor

@kvas-damian thank you for your proposal, it's always interesting to hear about pain points in the practice of developing Ember applications.
Like some of the commenters, I also want to suggest that you create an addon to solve this use case, and then share it with the community. This is usually the recommended that for several reasons:

  • It enables you to explore the problem space and validate the approach
  • It strengthens the proposal by virtue of being able to say "install this addon to try it"
  • You can argue for including the addon in the default blueprint, instead of adding to Ember's API

In this case, I believe that between the new router service and FastBoot that you have adequate APIs to implement this via addon.

@mmun mmun added the T-routing label Aug 1, 2017
@mehulkar

mehulkar commented Apr 1, 2020

Copy link
Copy Markdown

@villander didn't you just do some work on linking to ember engines? Would you mind linking that here (if i remember correctly)

@buschtoens

Copy link
Copy Markdown
Contributor

@villander

Copy link
Copy Markdown

@mehulkar we have also Linking Between Engines RFC that based our external API for external routes.

Everything mentioned by @buschtoens follows this RFC and current API already implemented in Ember Engines addon.

@wagenet

wagenet commented Jul 23, 2022

Copy link
Copy Markdown
Member

I'm going to agree with @locks's suggestion and, since this has been relatively inactive, go ahead close this. If someone still wants to advocate this, let me know and I'll see that it gets a more comprehensive review.

@wagenet wagenet closed this Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.