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

Use build-time project.isModuleUnification() instead of feature flag. #228

Merged

Conversation

cibernox
Copy link
Contributor

This allows classic apps to NOT ship the MU resolver code.

@cibernox cibernox force-pushed the use-build-time-module-unification-check branch 2 times, most recently from 367f1ef to 0087eff Compare March 13, 2018 11:59
This allows classic apps to NOT ship the MU resolver code
@cibernox cibernox force-pushed the use-build-time-module-unification-check branch from 0087eff to 2620d96 Compare March 14, 2018 15:37
@cibernox
Copy link
Contributor Author

I tested this in MU and non MU apps and I discovered a but, bit it's working now.

}, resolverConfig.features);
},

init: function() {
this._super.init.apply(this, arguments);
this.options = this.options || {};

if (process.env.EMBER_RESOLVER_MODULE_UNIFICATION) {
this.project.isModuleUnification = function () {
Copy link
Member

Choose a reason for hiding this comment

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

This seems fairly odd to me. Why do we have to monkey patch this?

Copy link
Contributor Author

@cibernox cibernox Mar 15, 2018

Choose a reason for hiding this comment

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

For testing the MU scenario in Travis only. See https://github.com/ember-cli/ember-resolver/blob/master/config/ember-try.js#L17
Any suggestion on an alternative approach? I find this relatively clean.

@mixonic mixonic merged commit 54356ad into ember-cli:master Mar 16, 2018
@mixonic
Copy link
Member

mixonic commented Mar 16, 2018

Thank you @cibernox!!! This is great.

@cibernox cibernox deleted the use-build-time-module-unification-check branch March 16, 2018 17:44
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

3 participants