ember-engines icon indicating copy to clipboard operation
ember-engines copied to clipboard

[RFC] Provide `externalRouter` to Engines

Open villander opened this issue 3 years ago • 5 comments

Detailed design

Regarding the #587 and the PR already implemented #669 as a team we discussed and agreed in one of our monthly meetings on February 12 of this year to follow these steps:

  • [x] Deprecate current usage of router:service() by registering a stub service that proxies router calls to the root app router service - #764

  • [x] Create a deprecation documentation page about the router service on ember-engines.com. Provide suggestions for how to register the host's router as hostRouter and/or the root router as appRouter. However, we don't want to automatically create either of these services in an engine - the decision should be the developer's. - https://github.com/ember-engines/ember-engines.com/pull/111

  • [ ] Create engine-specific external router and make it available on-demand for the users

  • [ ] Consider allowing an experimental build flag to allow usage of an alpha engine-specific router which we can iterate upon.

We want a shorten externalRouter to just external to be more concise than support all the *External methods. For example: we would like to have - router.external.urlFor, router.external.isActive and etc.

To do that, we need to map all externalRoutes allowed and avoid the router.external to access prohibited routes

// super-blog/addon/engine.js
export default class SuperBlog extends Engine {
  // ...
  dependencies = {
    externalRoutes: [
      'home',
      'settings',
    ]
  };
}

Also, we need to add a flag to enable this feature on-demand:

// super-blog/index.js
const { buildEngine } = require('ember-engines/lib/engine-addon');

module.exports = buildEngine({
  name: 'super-blog',
  lazyLoading: {
    enabled: true
  }
  ...
  externalRoute: {
    enabled: true,
  }
});

Usage

Option A) External namespace

import Route from "@ember/routing/route";
import { action } from '@ember/object';
import { inject as service } from '@ember/service';

export default class YourRoute extends Route {
  @service router;
  
  @action
  goHome() {
    this.router.external.transitionTo('home');
  }
});

Option B) Route helper

import Route from "@ember/routing/route";
import { action } from '@ember/object';
import { inject as service } from '@ember/service';
import { external } from 'ember-engines/helper'

export default class YourRoute extends Route {
  @service router;
  
  @action
  goHome() {
    this.router.transitionTo(external('home'));
  }
});

We must keep the eyes on this RFC that deprecates the no longer needed *External methods on controllers and routes. - https://github.com/emberjs/rfcs/pull/674

Drawbacks

This RFC introduces the new concept of engines, which increases the learning curve of the framework. However, I believe this issue is mitigated by the fact that engines are opt-in packaging around existing concepts.

In the end, I believe that "engines" are just a small API for composing existing concepts. And they can be introduced at the top of the conceptual ladder, once users are comfortable with the basics of Ember, and only for those working on large teams or distributing addons.

Alternatives

Keep the old mindset from the first Engines RFC and move forward with the #669 adding *External methods.

Unresolved questions

Further, consider an external routing DSL to replace *External suffixed methods. Odds are that any solution will require a framework-level RFC since it will need to be understood by framework-level concerns such as LinkTo.

cc: @dgeb @rwjblue @buschtoens

villander avatar Jun 20 '21 23:06 villander

Thanks for keeping progress on the router moving, @villander.

I'm unclear on how this proposal intersects with:

Consider allowing an experimental build flag to allow usage of an alpha engine-specific router which we can iterate upon.

Are you seeing the proposal in this issue (i.e. router.external) as separate from the alpha router we discussed experimenting with?

My impression was that the alpha router would be used to experiment with all engine-specific routing concepts, including external routes.

dgeb avatar Jul 09 '21 13:07 dgeb

Are you seeing the proposal in this issue (i.e. router.external) as separate from the alpha router we discussed experimenting with?

No! it's the same, we going to release the engine-specific (i.e. route.external) with the build flag as an experimental feature. It's only a breakdown

@dgeb what I understood so far is that we going to use engine-specific including all external routes passed down to Engines. Since the engine-specific router, like the *External methods will have access only for externalRoutes as usual. Is that ok for you?

villander avatar Jul 09 '21 14:07 villander

No! it's the same, we going to release the engine-specific (i.e. route.external) with the build flag as an experimental feature. It's only a breakdown

Gotcha, thanks for clarifying.

So I think we really have two alternatives on the table for handling external routes:

A) The one described in this RFC, in which an external namespace on the router service contains alternative external-route-specific methods.

B) An external route helper (or perhaps just external: string prefix) for namespacing external routes that can be used directly with the engine's router service and the standard LinkTo helpers.

Perhaps the best path forward will be to fill out option B as an RFC similar to this one, and at the same time fill in any gaps here for option A. Hopefully, we'll then be in a good position to decide if one approach is clearly better. If so, we can implement that behind the experimental flag. If not, perhaps we implement them both for comparison purposes in separate branches and then decide?

It will be good to talk this over in our monthly meeting and hear from @rwjblue, @buschtoens, and others who are interested in this space.

dgeb avatar Jul 09 '21 15:07 dgeb

Will this allow the engine router to route to external routes without the necessity of mapping external routes. Our app has a ton of routes. We use engines to share the same experience across multiple routes. Initially we started mapping external routes but this created a bit bloat in the code for little benefit as we do not use engines as a "separate app".

So back to the question will this allow us to

this.router.external.transitionTo('home.foo.bar')

where home.foo.bar is a route in the consuming (parent) app?

lvegerano avatar Jul 21 '21 14:07 lvegerano

Will this allow the engine router to route to external routes without the necessity of mapping external routes

It's not true Luis! As you can see on the proposal the routes continue to be declared on dependencies as externalRoutes.

villander avatar Aug 06 '21 15:08 villander