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

Allow mounting routeless engines with a bound engine name #15015

Merged
merged 1 commit into from
Mar 18, 2017

Conversation

mike183
Copy link
Contributor

@mike183 mike183 commented Mar 14, 2017

This PR builds upon the work in #14967, introducing the ability to pass bound properties to the {{mount}} syntax, enabling the dynamic mounting of "routeless" engines.

This also resolves ember-engines/ember-engines#117

cc/ @rwjblue

@rwjblue
Copy link
Member

rwjblue commented Mar 14, 2017

LGTM

@dgeb / @chancancode / @trentmwillis - Any objections?

Copy link
Member

@trentmwillis trentmwillis left a comment

Choose a reason for hiding this comment

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

Overall, I'm good with it. For bad input though, I'd like to see feedback given to the user.

return this._lastDef;
return this._lastDef;
} else {
return null;
Copy link
Member

Choose a reason for hiding this comment

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

IMO this should log if the argument isn't a string/undefined/null.

Copy link
Member

Choose a reason for hiding this comment

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

ya, lets have it assert

@mike183
Copy link
Contributor Author

mike183 commented Mar 18, 2017

@trentmwillis Thanks for the review, I've added an assertion to check that the provided engine name is valid and also added a test to go along with it.

cc/ @rwjblue

Assert when an invalid engine name is passed to {{mount}}

Fix {{mount}} tests
@rwjblue rwjblue merged commit 286af16 into emberjs:master Mar 18, 2017
@mike183
Copy link
Contributor Author

mike183 commented Mar 18, 2017

🎉

@mike183 mike183 deleted the bound-mount-syntax branch March 18, 2017 15:57
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.

Route less engine mounting with a variable
3 participants