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.

Sign up for GitHub

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

Jump to bottom

<named slot> is not a function #148

Closed
pete-gi opened this issue Jan 25, 2024 · 6 comments
Closed

<named slot> is not a function #148

pete-gi opened this issue Jan 25, 2024 · 6 comments
Assignees
@thetutlage
Labels
Type: Question Needs clarification

Comments

@pete-gi
Copy link

pete-gi commented Jan 25, 2024

Package version

6.0.1

Describe the bug

I want to make a layout which can be extended by other .edge files.

When we had @layout and @section everything was easy. We could set a default value for the section and even extend it with @super. Now, when having to use @component, things are bit different, but to the point.

Consider a really simple layout with two slots - named head and default main:

<head>
  {{{ await $slots.head() }}}
</head>
<body>
  <main>
    {{{ await $slots.main() }}}
  </main>
</body>

When using it in another file this way:

@layout.view.base()
  Some content
@end

or with the old syntax:

@component('components/layout/view/base')
  Some content
@end

We get the $slots.head is not a function error.

I must specifically define an empty named slot content for it to work:

@layout.view.base()
  @slot('head')
  @end
  Some content
@end

I want to be able to add the slot content optionally. It would also be awesome to be able to have default slot content.

Reproduction repo

No response
@thetutlage
Copy link
Member

Just check if slot exists using

@if($slots.head)
<head>
  {{{ await $slots.head() }}}
</head>
@end

@thetutlage thetutlage self-assigned this Jan 26, 2024
@thetutlage thetutlage added the Type: Question Needs clarification label Jan 26, 2024
@pete-gi
Copy link
Author

pete-gi commented Jan 26, 2024

Ok, I can. But it's a huge step back. I know I don't have to explain to you how it worked in Adonis v5, but it requires more code to achieve the same result.
Templating engine lacks now functionality that worked before pretty well. It's a regression.

@RomainLanz
Copy link
Member

But it's a huge step back

This is a strong statement for one additional line of code.

It totally makes sense to verify that a slot exists before trying to use it. In fact, before, you were generating bad HTML because you had an empty <head> tag.

@pete-gi
Copy link
Author

pete-gi commented Jan 26, 2024

In fact, before, you were generating bad HTML because you had an empty tag.

That's just an example with the smallest amount of HTML anyone needs in this issue.

This is a strong statement for one additional line of code.

One line of code for every slot. You don't know what's the scope of the projects people are working on.

But.
WebComponents don't need conditionals for slots. Vue doesn't need.
Twig for PHP has blocks like the sections before and doesn't need if statement. Laravel Blade the same.

I know I can make a global function or @component that can take care of this conditional for me. I'm working with Adonis since v5 released and having to write more code for something that was already there in previous version for years is a regression.

@thetutlage
Copy link
Member

Using components for layouts is a very common practice. Just search for the same in the Vue ecosystem or in the React ecosystem.

Yes, they work different from layouts in first place and you just have to find the right pattern with them.

But anyways, we won't be adding layouts back now. So the debate around the advantages and disadvantages won't help much.

@pete-gi
Copy link
Author

pete-gi commented Jan 27, 2024

Using components for layouts is a very common practice. Just search for the same in the Vue ecosystem or in the React ecosystem.

I work with them on the daily basis for 5 years, I don't have to search for it.

I just wanted to not use the if statement for checking whether the slot exists. That was the issue. You removed it from previous version of Edge.
But ok, you're not doing anything in this matter, I do accept this and go back to something that works as it should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thetutlage thetutlage

Labels
Type: Question Needs clarification
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@thetutlage @RomainLanz @pete-gi