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

same named block in nesting extends file #2436

Closed
Secbone opened this issue Jun 28, 2016 · 8 comments · Fixed by #2792
Closed

same named block in nesting extends file #2436

Secbone opened this issue Jun 28, 2016 · 8 comments · Fixed by #2792

Comments

@Secbone
Copy link

Secbone commented Jun 28, 2016

page.pug

extends layout.pug

block foo
    h1 page text

block widgets
    include widget.pug

widget.pug

extends widget_base.pug

block foo
    h3 widget text

the block foo in page.pug will be <h3>widget text</h3>.

pug 2.0.0-alpha3

is this a bug?

@Secbone Secbone changed the title same name block in nesting extends file same named block in nesting extends file Jun 28, 2016
@ForbesLindesay
Copy link
Member

Did it have the same behaviour in jade 1.11? I've tried to keep consistency with the quirks of jade for this first release, and then change to something more clear and well defined in v3.

@Secbone
Copy link
Author

Secbone commented Jul 24, 2016

@ForbesLindesay I test the case in Jade, It is rendered <h1>page text</h1> in both places. It seems a little different between jade and pug.

@TimothyGu
Copy link
Member

With pug 2.0.0-beta4, and these files:

//- a.pug
extend b.pug

block a
  | a
//- cat b.pug
extend c.pug

block a
  | b
//- cat c.pug
block a
  | c

and

pug.renderFile('a.pug')

The result is

a

Try updating your Pug version.

@Secbone
Copy link
Author

Secbone commented Jul 25, 2016

@TimothyGu that is not the problem. I test the case with pug 2.0.0-beta4 :

//- main.pug
extends main_base.pug
block a
    | Main A
block b
    include other.pug

//- other.pug
extends other_base.pug
block a  // this block is extends from other_base.pug, but with the same name in main_base.pug
    | Other A

I think it will render

Main A
Other A

but the result is

Other A
Other A

and I also test with jade 1.11.0, the result is

Main A
Main A

@TimothyGu TimothyGu added this to the 2.0.0 - Modules and Plugins milestone Jul 25, 2016
@TimothyGu TimothyGu added the bug label Sep 4, 2016
@ForbesLindesay
Copy link
Member

@Secbone could you add a test case demonstrating this bug? See #2687 for an example of doing this. The snapshot files get automatically generated when you run npm test and then you can edit them to show the output you expected. Once we have a failing test, this should be much easier for us to fix.

@Secbone
Copy link
Author

Secbone commented Jan 29, 2017

@ForbesLindesay OK, I will create a PR.

@kylekatarnls
Copy link
Contributor

Hi, I'm about to implement named block in https://github.com/phug-php/phug (the PHP port of Pug that will both replace pug-php and tale-jade). Is everyone agree the expected behaviour is:

Main A
Other A

As in the unit test Secbone suggest. So if it's planned to be fixed before 2.0.0 release, I will yet implement it the Secbone way in our PHP project.

Thanks,

@ForbesLindesay
Copy link
Member

@kylekatarnls yes, I agree that should be the output in this instance. I propose the following rule:

If an included template uses extends, the blocks in that template will be ignored by the template that includes it. i.e. a template being included can either use blocks to interact with the primary template, or it can be part of its own completely separate inheritance chain, but not both.

Does this make sense to people?

The consequence of that is that:

layout.pug

h1 layout
block foo

page.pug

extends layout.pug

block foo
    h2 page text

block widgets
    include widget.pug

widget.pug (note no extends in this file)

block foo
    h3 widget text

would produce:

<h1>layout </h1>
<h3>widget text</h3>"

ForbesLindesay pushed a commit that referenced this issue May 1, 2017
ForbesLindesay added a commit that referenced this issue May 1, 2017
* Add regression tests for #2436

* Update snapshot to match user's typical expectations.

* Remove named blocks from included templates that extend

[fixes #2436]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants