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

conditionally add bindings related methods to meta #16355

Merged
merged 1 commit into from
May 28, 2018

Conversation

bekzod
Copy link
Contributor

@bekzod bekzod commented Mar 10, 2018

ENV._ENABLE_BINDING_SUPPORT is not enabled there is no need to have such methods in meta ?

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.

Seems good, but we should ensure the tests in Ember-2-legacy addon still pass. I recall there being a race condition where the addons code executes after Ember is evaluated which would make this change break the addons ability to add binding Support...

@krisselden
Copy link
Contributor

@rwjblue addons should get a chance to add to the env, I think module unification if I recall is pulling config too early and caching it, we should ensure the env does get baked until all addons been initialized.

@rwjblue
Copy link
Member

rwjblue commented Mar 24, 2018

@krisselden - I completely agree with what you said, but it has no relationship to this PR (or my prior comment).

@bekzod bekzod force-pushed the conditionally-bindings branch 2 times, most recently from db261eb to 7f6e102 Compare May 21, 2018 15:47
@bekzod
Copy link
Contributor Author

bekzod commented May 28, 2018

@rwjblue this is updated with svelting flags

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.

Awesome, thank you!

@rwjblue rwjblue merged commit 1b83765 into emberjs:master May 28, 2018
@bekzod bekzod deleted the conditionally-bindings branch May 28, 2018 14:47
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.

3 participants