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

[CLEANUP] Remove {{render}} #15915

Merged
merged 2 commits into from Dec 13, 2017
Merged

[CLEANUP] Remove {{render}} #15915

merged 2 commits into from Dec 13, 2017

Conversation

thoov
Copy link
Member

@thoov thoov commented Dec 1, 2017

Related: ember-2-legacy PR

Apart of: #15876

@thoov
Copy link
Member Author

thoov commented Dec 1, 2017

@rwjblue I don't have a great way of getting rid of these other files other than putting a comment on them for future removal. Do you have any ideas?

Right now I'm just making sure that we don't include render is what we want, just not sure if we can clean this up any more

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.

The approach seems good to me, I don't think there is much more we can do to reduce payload size while still supporting what we need...

@@ -65,7 +65,11 @@ export function registerMacros(macro: any) {
export function populateMacros(blocks: any, inlines: any) {
inlines.add('outlet', outletMacro);
inlines.add('component', inlineComponentMacro);
inlines.add('render', renderMacro);

if (ENV._ENABLE_RENDER_SUPPORT === true) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this ENV is not imported (thats what one of the test failures indicated at least)..

Copy link
Member

Choose a reason for hiding this comment

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

@thoov - I think fixing this will fix CI.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rwjblue Yep that fixed it. Should be good to go now

@rwjblue rwjblue merged commit ab5e69e into emberjs:master Dec 13, 2017
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.

None yet

2 participants