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

Fix issue when using named blocks with wrapped components (e.g. @ember/component) #1068

Merged
merged 12 commits into from
Apr 12, 2020
Merged

Fix issue when using named blocks with wrapped components (e.g. @ember/component) #1068

merged 12 commits into from
Apr 12, 2020

Conversation

locks
Copy link
Contributor

@locks locks commented Apr 6, 2020

No description provided.

rwjblue
rwjblue reviewed Apr 7, 2020
View reviewed changes
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a test for:

<this.Foo>
  <:bar>
   Stuff!!!
  </:bar>
</this.Foo>

packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts Outdated Show resolved Hide resolved
packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts Outdated Show resolved Hide resolved
packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/shared.ts Outdated Show resolved Hide resolved
packages/@glimmer/opcode-compiler/lib/utils.ts Outdated Show resolved Hide resolved
packages/@glimmer/runtime/lib/compiled/opcodes/component.ts Outdated Show resolved Hide resolved
packages/@glimmer/runtime/lib/compiled/opcodes/component.ts Outdated Show resolved Hide resolved
packages/@glimmer/runtime/lib/compiled/opcodes/component.ts Outdated Show resolved Hide resolved
packages/@glimmer/runtime/lib/compiled/opcodes/component.ts Outdated Show resolved Hide resolved
packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts Show resolved Hide resolved
packages/@glimmer/runtime/lib/compiled/opcodes/expressions.ts Show resolved Hide resolved
@locks locks force-pushed the named-blocks-curly branch 2 times, most recently from 8f6edcc to a319d23 Compare April 9, 2020 09:46
@rwjblue rwjblue marked this pull request as ready for review April 10, 2020 15:37
@rwjblue rwjblue changed the title add named block tests for emberish curly component Fix issue when using named blocks with wrapped components (e.g. @ember/component) Apr 10, 2020
locks and others added 12 commits April 10, 2020 18:30
@locks
add named block tests for emberish curly component
414b052
@rwjblue @wycats @locks
WIP - Test passes...
e5b5789 
Co-authored-by: Yehuda Katz <[email protected]>
Co-authored-by: Ricardo Mendes <[email protected]>
@rwjblue @locks
Fix failure with HasBlockParams
13f887d
@locks
remove commented code
afc5a56
@locks
make it visually similar to SetNamedVariables
c85c03b
@locks
derive names at initialization time
3ba3de6
@locks
add names property to NamedBlocks interface
88d151e
@locks
use checker in HasBlock
e56ba60
@rwjblue @locks
Avoid string concat in Ops.SetBlocks
234ec45 
Co-authored-by: Ricardo Mendes <[email protected]>
@rwjblue @locks
Cleanup Op.HasBlock to leverage CheckUndefinedReference
06f67b6 
Co-authored-by: Ricardo Mendes <[email protected]>
@rwjblue @locks
Fixup linting error.
aa8faec 
Co-authored-by: Ricardo Mendes <[email protected]>
@rwjblue @locks
Add more tests.
4e70d1f 
Co-authored-by: Ricardo Mendes <[email protected]>
@locks locks force-pushed the named-blocks-curly branch from acf76cb to 4e70d1f Compare April 10, 2020 17:30
@locks
Copy link
Contributor Author

locks commented Apr 10, 2020 •
edited
Loading

Ember master tests pass with these changes linked.

@locks locks requested a review from pzuraq April 10, 2020 17:48
@rwjblue rwjblue requested a review from krisselden April 11, 2020 15:58
@rwjblue rwjblue merged commit 18b3400 into master Apr 12, 2020
@rwjblue rwjblue added the bug label Apr 13, 2020
@locks locks deleted the named-blocks-curly branch May 28, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lifeart lifeart lifeart left review comments

@rwjblue rwjblue rwjblue approved these changes

@pzuraq pzuraq pzuraq approved these changes

@krisselden krisselden Awaiting requested review from krisselden

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@locks @rwjblue @pzuraq @lifeart