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

Use a custom div instead of duplicating the block #10

Open
mupkoo opened this issue Jul 24, 2018 · 7 comments
Open

Use a custom div instead of duplicating the block #10

mupkoo opened this issue Jul 24, 2018 · 7 comments

Comments

@mupkoo
Copy link

mupkoo commented Jul 24, 2018

The idea is to have a component instead of an AST transform. This way there will be no need to have the same block twice.

Something in the lines of

import Component from '@ember/component';

export default Component.extend({
  renderInPlace: false,

  _parentElement: computed('parentElement', 'renderInPlace', function () {
    if (this.renderInPlace) {
      return this.element.querySelector('[data-render-in-place']);
    } else {
      return this.parentElement;
    }
  })
});
<div data-render-in-place></div>

{{#-in-element parentElement}}{{yield}}{{/-in-element}}

or maybe even something like. The bottom code works although I am not sure if it will cause any problems.

import Component from '@ember/component';

export default Component.extend({
  renderInPlace: false,

  _parentElement: computed('parentElement', 'renderInPlace', function () {
    if (this.renderInPlace) {
      return this.element;
    } else {
      return this.parentElement;
    }
  })
});
@cibernox
Copy link
Contributor

The reason for the AST transform is that it has zero runtime cost, unlike a component. Is there a reason that magically duplicated block is a problem for you?

@mupkoo
Copy link
Author

mupkoo commented Jul 24, 2018

Sometimes the block can contain bigger templates than a single component, which in a way will increase the template size. And although this can be avoided by creating a component and using it inside the block, I can see a lot of people just putting a lot of presentation logic inside there.

I guess it is a micro optimisation - template size vs runtime cost. It was just an idea that I wanted to share.

@cibernox
Copy link
Contributor

It makes sense. There is a middle ground thing we can do it would improve things in the most common use case.

The block is duplicated because of the renderInPlace option, which is often not used ({{maybe-in-element el}}) or set to a static value ({{maybe-in-element el false}}). We could make the AST detect those cases and avoid duplicating the block for those.

@mupkoo
Copy link
Author

mupkoo commented Jul 24, 2018

That is a nice idea. I can create a PR towards the end of the week.

@mupkoo
Copy link
Author

mupkoo commented Jul 24, 2018

Now when I think about it why would you use {{ maybe-in-element el false }} instead of {{ -in-element el }}

@cibernox
Copy link
Contributor

None really, other than consistency. At the end, using something that starts with {{- is kind of scary, but it would be identical.

@rwjblue
Copy link

rwjblue commented Apr 30, 2020

The reason for the AST transform is that it has zero runtime cost, unlike a component. Is there a reason that magically duplicated block is a problem for you?

FWIW, this not zero cost, it is double the bytes of whatever the block. I think the modern suggestion would be to make a template only component that does:

{{#if @renderInPlace}}
  {{yield}}
{{else}}
  {{#in-element @destinationElement insertBefore=null}}
    {{yield}}
  {{/in-element}}
{{/if}}

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

No branches or pull requests

3 participants