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

Fixed bug with page bemhtml wrapping in production mode #309

Merged
merged 1 commit into from
Nov 21, 2013
Merged

Fixed bug with page bemhtml wrapping in production mode #309

merged 1 commit into from
Nov 21, 2013

Conversation

blond
Copy link
Member

@blond blond commented Nov 12, 2013

Resolved #308

@tadatuta
Copy link
Member

// cc @veged @indutny

@indutny
Copy link
Contributor

indutny commented Nov 20, 2013

LGTM, if works.

@narqo
Copy link
Member

narqo commented Nov 20, 2013

А это точно не баг в bem-xjst? Почему шаблон (условный) зацикливается от конструкции

applyCtx([
  { elem : 'doctype' },
  { elem : 'root', content: this.ctx }
])

@indutny
Copy link
Contributor

indutny commented Nov 20, 2013

applyCtx(..) => applyNext({ _mode: '', ctx: ... }), а applyNext теперь работает только на глубину рекурсии 1. Так было изначально задумано, но, по ошибке, работало иначе. В bem-bl это уже не изменить (ломаются шаблоны), так что теперь это будет в bem-core.

По поводу этого PR, тут this.ctx перекидывается глубже, и рекурсия вернет нас к нему опять, а потом он снова перекинется глубже.

@veged
Copy link
Member

veged commented Nov 20, 2013

по идее вместо дополнительного local можно использовать присваивание внутри applyCtxapplyCtx({ … }, { 'ctx._wrapped': true }) (@indutny поправь меня, если я ошибаюсь)

@indutny
Copy link
Contributor

indutny commented Nov 20, 2013

Нельзя, это поставит флаг _wrapped вот такому массиву:

[
  { elem : 'doctype' },
  { elem : 'root', content: this.ctx }
]

@tadatuta
Copy link
Member

@veged вольешь?

veged added a commit that referenced this pull request Nov 21, 2013
Fixed bug with page bemhtml wrapping in production mode
@veged veged merged commit 277f362 into bem:v1 Nov 21, 2013
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.

5 participants