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

Introduces island-outlet #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xcambar
Copy link

@xcambar xcambar commented Nov 4, 2016

⚠️ WIP - tests missing

My apologies if that has already been discussed.

This PR introduces the concept of island-outlet. An island-outlet behaves like a typical outlet: it is a named placeholder for content. It allows to define in the DOM a number of locations for which the content will be dictated by the blocks yielded within the {{island-outlet}} components with the same name.

As an example, here's the HTML I had to deal with:

<div id="ember-application">
  <div data-component="app-navbar"/>
  <div class="container">
    <!-- 
           Here we want to display the contents of the route template here
           instead of a specific component.
           For this, we use `[data-outlet="main-outlet"]` below
    -->
    <div data-outlet="main-outlet"/>
  </div>
  <div data-component="app-footer"/>
</div>

and the following application.hbs:

{{outlet}}
{{ember-islands}}

Without using {{ember-outlet}}, I can not have the {{outlet}} to render at the correct place in the document (it will append to #ember-application).

With this PR, I can rewrite the template as follows:

{{#island-outlet "main-outlet"}}
  {{outlet}}
{{/island-outlet}}
{{ember-islands}}

and see my outlet rendered at the expected place.

If you welcome the idea, there's a lot of room for discussion and improvement (notably regarding the dependency of ember-wormhole), but for the moment, I simply wanted to share a draft on the concept.

Hope you'll like it!

@mitchlloyd
Copy link
Owner

Thanks for putting this PR together. My first impression is that this feature does fit with the purpose of Ember Islands.

One concern I have is that this is so close to typical usage of ember-wormhole that I wonder whether the abstraction is worth it or if it would be better to direct users to ember-wormhole.

<div class="container">
  <div id="main-outlet"></div>
</div>
{{#ember-wormhole to="main-outlet"}}
  {{outlet}}
{{/ember-wormhole}}

My other concern is that this is conceptually different from the existing feature. Currently I'm trying to build a mental model where the static template is asking for the content that it wants to render. Using ember-wormhole is generally a different model where the Ember app is pushing content into the static template.

Perhaps a more ideal API would be something like this:

<div class="container">
  <div data-route-path="posts/edit"></div>
  <div data-route-path="user/profile"></div>
</div>

Allowing the developer to arbitrarily render routes. But I'm not sure if this is feasible.

So this is my long-winded way of saying I need some time to think more about this 😄 . There is a lot in the mix with the ideas of routable components and presumably some forthcoming API for rendering Glimmer components without the rest of Ember. I'm hoping I'll have a lot more information to work with soon. In the mean time it could also make sense to merge a feature like this and then try to come up with a more comprehensive solution for a 2.0 version.

Let me know what you think.

@xcambar
Copy link
Author

xcambar commented Nov 4, 2016

I share your concerns about the similarity with the basic usage of ember-wormhole.

The only thing I can say in favour of this new API is my own story, highlighting some (hopefully interesting) Developer Experience facts:
I helped transition some apps to Ember, consistently having ember-islands in my toolset as a placeholder for Ember components. Every time, as the Ember app grows, comes a moment when I'd rather render the main {{outlet}} than statically declare the components in the HTML.
See the slides of a talk I've made recently about that (video soon): https://xcambar.github.io/emberfest_2016
So, the question here (from my perspective), can be solved by this question: "Does it make for a reasonable use case?" For me, this feature has somehow always been on my path.

As for your alternative, I don't feel confident about [data-route-path] because it suggests we can render 2 routes at the same time.

I am really looking forward to being able to use routable components with ember-islands too yet as you suggested, the impact is still very much unknown.

@mitchlloyd
Copy link
Owner

mitchlloyd commented Nov 4, 2016

To be clear I'm leaning toward merging this feature because it seems to fit with the exact vision of this addon. If you've been using Ember Islands a lot recently then you probably have better perspective than I do at this point. I just want to stew on it for a day or two to make sure I'm not missing something.

Is there a video up for that talk yet? I'd like to see it.

When I wrote that data-route-path example I was trying to emphasize that the route path isn't necessarily tied to the page that the user is visiting. However, you're right that rendering 2 different pages at once is not a path toward a full Ember app. I'm assuming that you're co-opting the URL to choose which route to render (as per the README), but it would be nice if you didn't have to line up the URL on your web-server with the URL in your Ember app. Again I'm not sure if that is feasible.

Even though the use case is clear, there's nothing really specific to Ember's outlets here so I wonder if that's a misnomer. In other words, this would be perfectly valid:

{{#island-outlet "main-outlet"}}
  Hello from an Island Outlet!
{{/island-outlet}}
{{ember-islands}}

This is why I'm trying to think of a way to flip this from "pushing from Ember app into my static page" to "pulling the Ember app into my static page" to fit the current mental model. However, maybe the answer here is that there should just be a general-purpose way to "push" content into the placeholders. In that case, maybe there are some naming changes that can better support that idea.

<div data-content-for="main"/>
{{#ember-islands "main"}}
  Hello from Ember Islands!
{{/ember-islands}}

This could be backwards compatible with the current approach, but we might want to add

{{ember-islands-components}}

to clarify the other use case.

@mixonic
Copy link
Collaborator

mixonic commented Nov 4, 2016

As outlet is an Ember internal it might be difficult to build {{ember-islands 'main'}} with similar functionality to {{outlet 'main'}}.

@mitchlloyd
Copy link
Owner

mitchlloyd commented Nov 4, 2016

@mixonic {{ember-islands 'main'}} has nothing to do with Ember outlets per se, which is why I was proposing the name change. This is roughy equivalent to:

{{#ember-wormhole to="main"}}
  Hi!
{{/ember-wormhole}}

@xcambar
Copy link
Author

xcambar commented Nov 5, 2016

(Video for the EmberFest talk is not ready yet. A previous version has been presented at EmberConf this year: https://www.youtube.com/watch?v=-iL8ME7Ux2k)

@mitchlloyd I love the syntax you proposed
👍 for [data-content-for]
👍 for using block templates in {{ember-islands}}

In the current implementation, at most one {{ember-islands}} is really ever used per template.
Here, as there could more than one and because I tend to favour explicit behaviour, I imagine that only the unlabelled instance provides the current behaviour. Do you have another perspective?

Following on this idea, a template could be as follows, correct?

{{!--
  for `[data-content-for]`
--}}

{{#ember-islands "main"}}
  Hello from Ember Islands!
{{/ember-islands}}
{{#ember-islands "secondary"}}
  Hello as well!
{{/ember-islands}}

{{!--
  for `[data-component]`
--}}

{{ember-islands}}

@xcambar xcambar changed the title Introduces ember-outlet Introduces island-outlet Nov 5, 2016
@mitchlloyd
Copy link
Owner

@xcambar Yes, that's what I was thinking, but there might be some more name tweaking ahead :)

One more factor to throw into the mix is that Ember Canary currently has an -in-element helper would allow us to avoid adding Ember Wormhole as a dependency. Because this is a new feature we could require the current version of Ember to use it.

This helper currently undergoing some churn (see emberjs/ember.js#14472) but it would be great to use a purpose-made API for this.

Now we just need something like application.renderComponent('my-component', element) in Ember and this addon would be trivial :)

@xcambar
Copy link
Author

xcambar commented Jul 20, 2018

Hi,
Is this PR still relevant?
What would it take to revive the discussion?

@mitchlloyd
Copy link
Owner

@xcambar I think this idea is still relevant. The last API that you posted still looks good to me. Could you take a shot at implementing and explain it's "reason for being" in the README? Ideally we would use the newly public in-element helper. This polyfill seems good to support older versions.

Above and beyond (and maybe later) would be an attempt to use in-element for all of ember-islands which would make this library private-api-free. However the last time I looked into this, the fact that Ember does not have a splat operator or any way to send an object as attributes ("properties" in Glimmer terms) to a component meant that the API would not work as is for this library. I haven't really had my ear to the ground, so hopefully this has changed and in-element could replace all the private APIs in Ember Islands.

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

Successfully merging this pull request may close these issues.

3 participants