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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 0 additions & 53 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,59 +15,6 @@ This project provides the Ember resolver used by the following projects:
ember install ember-resolver
```

## Feature flags

This resolver package supports feature flags for experimental changes. Feature
flag settings change how `ember-resolver` compiles into an ember-cli's
`vendor.js`, so you should think of them as an application build-time option.

Feature flags are set in an application's `config/environment.js`:

```js
module.exports = function(environment) {
var ENV = {
'ember-resolver': {
features: {
EMBER_RESOLVER_MODULE_UNIFICATION: true
}
},
/* ... */
```

Note that you must restart your ember-cli server for changes to the `flags` to
register.

In the `ember-resolver` codebase, you can import these flags:

```js
import { EMBER_RESOLVER_MODULE_UNIFICATION } from 'ember-resolver/features';
```

### Current feature flags

#### `EMBER_RESOLVER_MODULE_UNIFICATION`

Ember [RFC #154](https://github.com/emberjs/rfcs/blob/master/text/0143-module-unification.md)
describes an improved resolution strategy and filename-on-disk
layout for Ember applications. To experiment with this feature
it must be enabled as described above, then use the `src/`
directory on disk. You can generate a new app that uses
this layout by using the following commands:

```
# Create a new app with the module unification blueprint
ember new my-app -b ember-module-unification-blueprint
```

This will create an app running a module unification layout from
the
[ember-module-unification-blueprint](https://github.com/emberjs/ember-module-unification-blueprint)
package. By default, this app will be correctly configured.

* It uses the `glimmer-wrapper` resolver.
* It builds an glimmer resolver config and passes it to the resolver.
* It starts with a `src/` based layout on disk.

## Configuration

To customize pluralization provide a `pluralizedTypes` object to your extended version of the Resolver in consuming app:
Expand Down
6 changes: 3 additions & 3 deletions ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ module.exports = function(defaults) {
exclude: [/^dummy/],
})];

let config = defaults.project.config();
let resolverConfig = config['ember-resolver'] || {};
let isModuleUnification = !!defaults.project.isModuleUnification &&
defaults.project.isModuleUnification();

if (resolverConfig.features.EMBER_RESOLVER_MODULE_UNIFICATION) {
if (isModuleUnification) {
testTrees.push('mu-trees/tests');
}

Expand Down
13 changes: 9 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var VersionChecker = require('ember-cli-version-checker');
var path = require('path');
var isModuleUnification;

module.exports = {
name: 'ember-resolver',
Expand All @@ -11,16 +12,20 @@ module.exports = {
var resolverConfig = config['ember-resolver'] || {};

return Object.assign({
/* Add default feature flags here */
EMBER_RESOLVER_MODULE_UNIFICATION: false
/* Add default feature flags here, for now there is none */
}, 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.

return true;
}
}
this._emberResolverFeatureFlags = this.emberResolverFeatureFlags();
isModuleUnification = !!this.project.isModuleUnification && this.project.isModuleUnification();

this.options.babel = {
loose: true,
Expand All @@ -47,7 +52,7 @@ module.exports = {
var MergeTrees = require('broccoli-merge-trees');
let addonTrees = [].concat(
this._super.treeForAddon.apply(this, arguments),
this._emberResolverFeatureFlags.EMBER_RESOLVER_MODULE_UNIFICATION && this._moduleUnificationTrees()
isModuleUnification && this._moduleUnificationTrees()
).filter(Boolean);

return new MergeTrees(addonTrees);
Expand Down
5 changes: 0 additions & 5 deletions tests/dummy/config/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ module.exports = function(environment) {
APP: {
// Here you can pass flags/options to your application instance
// when it is created
},
'ember-resolver': {
features: {
EMBER_RESOLVER_MODULE_UNIFICATION: !!process.env.EMBER_RESOLVER_MODULE_UNIFICATION
}
}
};

Expand Down