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

RFC: Engines #10

Merged
merged 4 commits into from
Apr 11, 2016
Merged

RFC: Engines #10

merged 4 commits into from
Apr 11, 2016

Conversation

tomdale
Copy link
Member

@tomdale tomdale commented Oct 25, 2014

RFC

with an engine could be accessed from the primary application like this:

```handlebars
{{authentication@login-form obscure-password=true}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

downside of this syntax is that it has no html compliant tag form (yet)

In theory something like:

<ember-namespace namespace="authentication">
   <login-form>
</ember-namespace>

or a block form, may introduce nice things

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanpenner How's that HTML-compliant tags align with (presumably) allowing tags with slashes after the Handlebars 2.0 update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now we will support slashes only via {{foo/bar}} syntax, as <foo/bar></foo/bar> is kinda crazy and not supported by HTMLBars currently. I am unsure it should be, but likely further thought is needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually seems like a somewhat serious issue. We're relying on HTMLBars syntax for something like: <my-component on-click={{action "foo"}}>, and there is no equivalent old-component syntax.

We will need to figure out a solution for nested tag names pretty soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: document.createElement("foo:bar") works (holdover from XML days, I suppose).

I think our solutions are going to end up being:

  • Template-wide imports (* or named)
  • Scoped imports (like XML namespaces or what Stef suggested)
  • Changes to the default namespace (would require html:div if HTML was desired).

I think that many solutions will work, and that as long as we have a clear, "one true story" for doing this, and it doesn't introduce excessive duplication or boilerplate, people will be able to live with it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its a bummer that @ wont work, as : already has meaning...

@stefanpenner
Copy link
Member

Elephant in the room is async. I believe engines can be a great enabler of this, but some concerns exist.

  • routing to to routes not-yet-loaded
  • link-to to routes not loaded
  • link-to to routes not loaded (where the routes have custom serialize)
  • foot-gun reopen + reopenClass invocations on classes who already have instantiated instances

Implicit vs Explicit namespaces within an engine.

Another, topic of discussion is, does one need to constantly use the namespace within the namespace, for example. Within authentication does one need to use:

this.controllerFor('authentication@foo');

or can we without ambiguity consistently infer the namespace and only do.

this.controllerFor('foo');

Engines all the way down.

In a perfect world, top level applications would also be engines and this would in "pure" style of engines all the way down. Does this make sense, is this tenable etc..

@tomdale
Copy link
Member Author

tomdale commented Oct 25, 2014

@stefanpenner Container keys used in the context of an engine should implicitly be in the same namespace; i.e. within authentication one can just say this.controllerFor('foo'); to get the authentication engine's foo controller.

@stefanpenner
Copy link
Member

@stefanpenner Container keys used in the context of an engine should implicitly be in the same namespace; i.e. within authentication one can just say this.controllerFor('foo'); to get the authentication engine's foo controller.

so this would imply a custom container (or namespace aware shared container). What about the resolver, does each engine use the same resolver?

I would love to work through some gnarly multi-container multi engine examples as I believe this will be more complicated then we will expect. Some early ideas @rwjblue and me through down https://github.com/rwjblue/container-resolver-namespaces

@lukemelia
Copy link
Member

I love where this is going, and I think it will be very helpful lots of teams. I think this RFC should include the mechanism(s) that would be available to for the application developer to modify classes and templates provided by the engine.

e.g. in the authentication scenario you described, there is an out-of-the-box login.hbs template. How would a developer provide her own template to be used instead? Same question for the login controller. And would this be on an engine-by-engine basis, or would there be an automatically supported convention for this?

@tomdale
Copy link
Member Author

tomdale commented Oct 25, 2014

@stefanpenner

so this would imply a custom container (or namespace aware shared container)

My gut feeling here is that you'd want some kind of container per engine, but the container for engines would just be a sort of proxy that ensured everything on the way out was namespaced.

So basically, if a controller inside an engine did this.controllerFor('foo'):

  1. controllerFor('foo') would do this.container.lookup('controller:foo')
  2. The engine's container would know its namespace, authentication, and proxy a call to the "parent" container with authentication@controller:foo (parent nomenclature is a little confusing here)

@tomdale
Copy link
Member Author

tomdale commented Oct 25, 2014

@lukemelia

e.g. in the authentication scenario you described, there is an out-of-the-box login.hbs template. How would a developer provide her own template to be used instead? Same question for the login controller. And would this be on an engine-by-engine basis, or would there be an automatically supported convention for this?

That seems like it should be up to the engine author to me. In the specific case of authentication, I can imagine the author building the login page out of a series of components that talk to a service. If you don't like the built-in login page, you can create your own page and re-configure the provided components. Or, if you want totally custom UI, you can just inject the service and talk to it directly from custom components.

@lukemelia
Copy link
Member

@tomdale wrote

That seems like it should be up to the engine author to me. In the specific case of authentication, I can imagine the author building the login page out of a series of components that talk to a service. If you don't like the built-in login page, you can create your own page and re-configure the provided components. Or, if you want totally custom UI, you can just inject the service and talk to it directly from custom components.

In this scenario, how would I tell the engine's login route to use the page that I crafted out of the engine's components? Does needing to change a small detail on the login screen mean I need to replace the template, route and router?

It would be awesome (though potentially a footgun) if there were a way to make it easy for engine authors to either explicitly provide extension points, or to have every piece of an engine overridable by a) providing a file at a corresponding path in your own app, e.g. under ext/engines/engine-name and/or b) registering your own implementation with the container.

@stefanpenner
Copy link
Member

@lukemelia it would seem like adding custom modules to engine/engine-name/templates/login.hbs would do the trick. As a 3rd party engine would actually exists in node_modules somewhere, so as long as engine/<name>/templates/login.hbs has a higher priority then node_modules/auth-plugin/engine/template/login.hbs when merging, it may "just work"

@stefanpenner
Copy link
Member

My gut feeling here is that you'd want some kind of container per engine, but the container for engines would just be a sort of proxy that ensured everything on the way out was namespaced.

ya, container + resolver pair per engine, with a global registry to enable escaping the current namespace.

@ef4
Copy link
Contributor

ef4 commented Oct 25, 2014

There may be good reasons to not pave an easy path for engines to escape their own namespace. What if we constrained their external interface to only be:

  • a set of routes they expose, which the rest of the application can transitionTo, link-to, etc.
  • a set of events that can bubble out of the engine up to the parent route of its mount point, just like normal route bubbling.

From the outside looking in, no new namespacing interface is needed because the user already namespaced these routes based on where they called mount and the value of the proposed path argument. Within the engine, every route would just be relative to the mount point.

The engine would not directly link-to or transitionTo routes outside its own namespace. It can get all the same effects by bubbling events out and letting the containing application (or the next engine up the stack!) deal with it. The engine couldn't do controllerFor, modelFor, etc outside its own scope, which I suspect is a good thing.

This is fairly analogous to the way components work. In this sense, engines are just super-components that happen to handle their own routing.

@tomdale
Copy link
Member Author

tomdale commented Oct 25, 2014

a set of events that can bubble out of the engine up to the parent route of its mount point, just like normal route bubbling.

As a heads up, we're considering moving away from a "bubbling events" model to one where consumers of components pass in callbacks that can be called (sort of like React but without the verbosity).

@juggy
Copy link

juggy commented Oct 25, 2014

I currently have an app that is broke down into multiple rails engine. Each rails engine has its own Ember namespace with routes, models, templates, etc. Here is for example the initializer for a CRM engine:

Crm = Ember.Namespace.create()

Ember.Application.initializer
  name: 'router:crm',

  initialize: (container, application)->
    router = container.lookup("router:main")

    # Load the CRM route classes
    for klassName, klass of Crm

      if klassName.indexOf("Route") > 0
        klassName = klassName.replace("Route", "")
        klassName = "route:" + klassName.dasherize().replace(/\-/g, ".")
      else if klassName.indexOf("Controller") > 0
        klassName = klassName.replace("Controller", "")
        klassName = "controller:" + klassName.dasherize().replace(/\-/g, ".")
      else
        continue

      application.register( klassName, klass )  

    # Load the paths into the router
    router.constructor.map ->
      @resource 'parties', ->
        @route('new')
        @route('edit', {path : '/:id/edit'})
        @resource 'parties.show', {path : '/:id'}, ->
          @route('details', {path : '/details'})
          @route('activities', {path : '/activities'})

The first part of the initializer takes all the controllers/routes from the crm namespace and register them with the main application. The second part, well, is the router definition for the engine.

The drawback is that the main application currently does not know about the other namespaces and does not decide which routes get mounted.

Now the fun part is when you want to add routes to a 'dependent' engine. Let's say we have a project engine dependent on the crm engine. We would like to enhance a contact with a list of projects. The means:

  • change the model to add a relationship to projects
  • change the routes to handle /contacts/:id/projects
  • change the template to include the link to the new projects route

Here is the code for the project namespace initializer:

Projects = Ember.Namespace.create()
Ember.Application.initializer
  name: 'router:projects',

  initialize: (container, application)->
    router = container.lookup("router:main")

    # Load the Projects route classes
    for klassName, klass of Projects

      if klassName.indexOf("Route") > 0
        klassName = klassName.replace("Route", "")
        klassName = "route:" + klassName.dasherize().replace(/\-/g, ".")
      else if klassName.indexOf("Controller") > 0
        klassName = klassName.replace("Controller", "")
        klassName = "controller:" + klassName.dasherize().replace(/\-/g, ".")
      else
        continue

      application.register( klassName, klass )  

    # Load the paths into the router
    router.constructor.map ->
       @resource 'parties', ->
         @resource 'parties.show', {path:"/:id"}, ->
           @route("projects", {path: "/projects"})

      @resource 'projects', ->
        @route('new')
        @route('edit', {path : '/:id/edit'})
        @resource 'projects.show', {path : '/:id'}, ->
          ...

The first part is the same as for the Crm engine can could be extracted for reuse. The second part loads the projects routes, but also add a new projects route in the parties route.

For the template, I create a new template with the exact same path and name that will be loaded after the crm template. It is a copy of the original template so it is not optimal, but there is no easy way of doing it otherwise. For what it is worth, rails uses the same "copy to extend" pattern for the views in engines.

As for the model, I just reopen the class within the project namespace and add the relationship. Same goes for new actions on the controllers, routes, etc.

The technique above works really well for now and if you can build upon that, that would be sweet. I hope this helps!

@bcardarella
Copy link
Contributor

I'm confused between the difference of this RFC and ember-cli's already existing addons. Addons are effectively rails-like engines already. Beyond the engine-specific container I see a lot of overlap.

@ef4
Copy link
Contributor

ef4 commented Oct 26, 2014

@bcardarella I mostly agree. The only thing addons can't do is add routes to the router map. Even that could be done today by exporting a mount function from the add-on that the user can call from their router map.

@bcardarella
Copy link
Contributor

@ef4 yup, ember-admin is kind of doing this already but you have to opt-into it: https://github.com/dockyard/ember-admin#usage

@rwjblue and I have discussed a few time renaming the addons to engines, and I think the scope of ability of addons have outgrown the term "addon" itself. I'm all for this RFC I just don't think there needs to be a separate effort.

@drogus
Copy link

drogus commented Oct 26, 2014

One thing that is not discussed in the proposal is handling links between an engine and an application. In the example of authentication, it's almost certain that you will need to link to login page of the engine from the host application, for example {{link-to "auth.login"}}. I'm not sure what should be the syntax and how the engine should be referred. In Rails there's an option to name an engine using :as option, like mount Authentication::Engine, :as => 'auth', which defaults to the lowercased and underlined version of the namespace. There are also other cases in this are:

  • calling hosted app's urls from an engine, in Rails it's done by using main_app helper
  • calling engine's urls from an engine - we need to decide if the namespace needs to be used to call urls or are templates in context of an engine treated specially, ie. they know about the namespace. In rails you don't have to use any namespace in such scenario, so the effect depends on the context, but I'm not sure if this would be a good option for Ember.js

Another thing to discuss is if an engine should be allowed to run without a host application. In Rails this idea was abandoned, because of various technical difficulties and design decisions that were made earlier, but it could be possible in Ember.js. The question is if this is something useful in Ember.js world and if it's hard to implement.

@lukemelia mentioned customisation, which is also something important. In my opinion overriding templates should work out of the box, so a properly namespaced template in the application should be rendered in the engine. On top of that, there should be a way to override any of the routes or controllers, possibly with ability to subclass them. For example I should be able to subclass LoginRoute and put the subclass in place of the LoginRoute. This is the least we can in order to not leave users with copying and pasting code or doing some other hacks.

Customisation is a hard topic in general, although it still looks much better than in Rails. Rails doesn't have any notion of view components out of the box. Instead, there are multiple ways to extend templates in engines, which mostly use a notion of annotations (you can annotate template with blocks, like header and then use it to insert content, for example before :header, "foo") or DOM manipulation (for example you can say `before '#header', 'foo').

@drogus
Copy link

drogus commented Oct 26, 2014

One more thing that came to my mind in the topic of router is mounting engines on a dynamic routes. In Rails, you can do something like that:

resource :users do
  mount Blog::Engine
end

which allows to link to blog in the following way: link_to "Posts", blog.posts(user).

@stefanpenner
Copy link
Member

which allows to link to blog in the following way: link_to "Posts", blog.posts(user).

this could be handled by subexpressions

@stefanpenner
Copy link
Member

I'm confused between the difference of this RFC and ember-cli's already existing addons. Addons are effectively rails-like engines already. Beyond the engine-specific container I see a lot of overlap.

i think the idea is to formalize and improve this part of ember to further facilitate ember-admin and similar efforts.

@tomdale we should add ember-admin to our use-cases to review.

@wagenet
Copy link
Member

wagenet commented Oct 27, 2014

I like this overall concept. Sounds like there are still some details to be resolved, but definitely a 👍 overall.

@ef4
Copy link
Contributor

ef4 commented Oct 27, 2014

As a heads up, we're considering moving away from a "bubbling events" model to one where consumers of components pass in callbacks that can be called (sort of like React but without the verbosity).

Then rephrase my comment to say "we should consider using the same kind of interface that components use." Whether that's bubbling events or callbacks, I think there's a potential win here in reusing the same concept.

@ef4
Copy link
Contributor

ef4 commented Oct 27, 2014

I have another use case I'd like to see covered by this feature, please consider adding it to your list:

I have several small chunks of application that get reused in multiple places. They're good candidates to become Components, except they're rich enough that they want to load their own models and have child routes of their own. I would love to be able to mount them in all the places where they should work, and have all the right child routes and Route classes wired up correctly.

For example, I have a rich editor for a "Doctor's Order". It needs to be able to pop up in multiple different contexts whenever someone clicks on related content. It benefits from being a real Route so that I can use the router to load the right models, and it has child routes too to handle more complex situations when the user may want to dive deeper into related data.

Today I'm stuck metaprogramming my router map and Routes so that I can define them all once and then apply them underneath each resource that may need them.

(I don't want to link to a single canonical resource for this, because I render it inside the currently active top-level tab of my application, and it retains a lot of surrounding context. These different contexts really are different conceptual routes.)

@knownasilya
Copy link
Contributor

Loving where this idea is going. I could totally use this today for breaking up complex apps, or reusing duplicate code across non-related apps. Would be super nice with ember-admin 👍

@atsjj
Copy link

atsjj commented Oct 27, 2014

Would this engine mechanism allow each composing app to attach itself to a different root element?

@knownasilya
Copy link
Contributor

Something like?

Router.map(function () {
  this.resource('admin', function () {
    this.mount('authentication', { path: 'auth' });
    // other resources/routes..
  });
});

@tomdale
Copy link
Member Author

tomdale commented Oct 27, 2014

@atsjj I personally don't need that feature. My thinking is that you'd still share a single root element, but maybe you can expand on your use case a little?

@atsjj
Copy link

atsjj commented Oct 28, 2014

@tomdale I wouldn't say I have a use case for such a feature, but rather have a situation that kinda happened. I've been adding some new functionality with Ember to an existing Rails app and I've had to work around the problem of my Ember project not owning the entire DOM.

I wouldn't want to suggest that this kind of feature be added into this RFC, but rather if this feature just might implicitly happen given my understanding that an Engine is just another (Ember) app inside an app.

All that being said, I'd be happy to share the actual case with you offline if you're interested.

Restore engines to tomdale/rfcs:master
@tomdale tomdale reopened this Sep 21, 2015
@tomdale
Copy link
Member Author

tomdale commented Sep 21, 2015

Sorry everyone 😬

@MiracleBlue
Copy link

Stop messing about @tomdale 😜

@pixelhandler
Copy link

@dgeb - Does this engines proposal take into consideration various versions of Ember running in one app?

Some background… based on our upgrade experience with Ember from early pre-1.0 versions up through v1.12 (became impossible to upgrade to 1.13 for us.) my current refactor effort involves breaking down a big app to many smaller apps. (I’ve had to do that before as well, sometimes a screen at a time.) And, based on previous history with JS library dependencies in big JS apps (e.g. multiple version of jQuery with noConflict) -

I came to the conclusion that I do NOT want another big Ember app that has all it’s dependencies requiring compatibility with a single Ember version, either due to template compiling requirements or other constraint. Basically many smaller apps should afford the abilty to run various apps on various versions of Ember.

@tschoartschi
Copy link

The idea of engines sounds great to me 👍 Since the initial RFC is from October 2014 I wanted to ask what the current state of engines is and if there is already a plan when it is supposed to land in Ember. I'm really excited about engines 😃

@dgeb
Copy link
Member

dgeb commented Dec 21, 2015

@tschoartschi Work has begun on engines. The Engine and EngineInstance base classes have been extracted in emberjs/ember.js#12685

I'm now working with @rwjblue on an ember-engines addon that extends these new base classes and others in order to get engines working in ember-cli. We're aiming to make this addon public by the end of January. After concepts have been proven in this addon, we'll gradually roll the functionality into ember and ember-cli.

@knownasilya
Copy link
Contributor

@dgeb where is this (addon) work happening?

@dgeb
Copy link
Member

dgeb commented Dec 21, 2015

@knownasilya the addon work is happening in a private repo that will be made public in the next few weeks (certainly before the end of January).

@tsteuwer
Copy link

tsteuwer commented Jan 5, 2016

@dgeb any update on when the repo will be made public?

@stefanpenner
Copy link
Member

@dgeb any update on when the repo will be made public?

@tsteuwer i suspect @dgeb's estimate to be still accurate

@dgeb
Copy link
Member

dgeb commented Jan 14, 2016

The ember-engines addon has been published. Although it should still be considered experimental, ember-engines does implement many of the concepts laid out in this RFC.

Feedback and contributions welcome.

@knownasilya
Copy link
Contributor

👏 for @dgeb and @rwjblue

@sandstrom
Copy link
Contributor

Great new! Awesome @dgeb and @rwjblue

@sglanzer-deprecated
Copy link

This is excellent news!

On Thu, Jan 14, 2016 at 1:59 PM sandstrom [email protected] wrote:

Great new! Awesome @dgeb https://github.com/dgeb and @rwjblue
https://github.com/rwjblue [image: ⛵]


Reply to this email directly or view it on GitHub
#10 (comment).

@oswaldoacauan
Copy link

awesome 👏

@danshapir
Copy link

Amazing job! Any thought on when will this addon will work on Beta/Release? Or even better, when will it be merged with Ember itself?

@stefanpenner
Copy link
Member

when will it be merged with Ember itself?

when its ready

@danshapir
Copy link

@stefanpenner well that's obvious. The question is do we have a timeline? Like ember 2.7 for example...

I would like to assist, but to get the funding from my company I need a timeline (not a strict one, just a give or take).

@mixonic
Copy link
Member

mixonic commented Mar 28, 2016

@danshapir Implementation work has largely been lead by @dgeb and @rwjblue. I suggest looking into issues at https://github.com/dgeb/ember-engines/issues and reaching out to them for context. If you have the time you can definitely make an impact.

@timkendall
Copy link

Has anyone put much thought into how or if one should define styles in an engine? The issue is styles from the parent app could easily smash those defined in the engine. It seems like something cool could be done with ember-css-modules.

The simple solution is to utilize the default, scoped behavior of CSS Modules to avoid conflicts altogether (or at least keep them to a minimum). Another option (that could be used in conjunction with this approach) is to have a way of defining styles in a component or controller that need to be fulfilled by the engine consumer. This would be pretty trivial seeing as how Ember-CSS-Modules already exposes a styles member on each controller and component.

Anyways I may be overthinking this but we recently ran into this problem in a project I was working on with engines.

@MiguelMadero
Copy link

@timkendall styles are a bit challenging due to the potential of overlaps, certainly tools or libs like css-modules can help, in our case, we're relying on well-scoped rules.

I don't think this problem is specific to engines, add-ons and any other vendor styles that you pull-in have the same issue. That said, engines, once we get lazy loading make the problem a bit harder. Today we could rely on load order and test it to make sure we get the expected behavior (e.g. app styles load after vendor styles or component styles are imported in a certain order). Then we could either fix the scoping or change the order to get the desired behavior. With lazy loading, the order depends on which routes are visited first, making the style order non-deterministic and extremely hard to test since the permutations can be really high even with a small number of engines (n!).

@dgeb dgeb merged commit 2d2cc97 into emberjs:master Apr 11, 2016
@dgeb
Copy link
Member

dgeb commented Apr 11, 2016

Merging finally 🎉

We should continue to refine Ember's public API related to engines via subsequent RFCs. And we are tracking issues and progress in the ember-engines addon.

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.