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

Add app.import to control legacy files, remove broccoli-bower. #562

Merged
merged 5 commits into from
May 5, 2014

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented May 1, 2014

Summary

This PR removes broccoli-bower since the bower.json spec doesn't provide enough information on exactly what should and shouldn't be imported, and it adds a new EmberApp.prototype.import method. EmberApp.prototype.import is used both internally and in your Brocfile to manage your dependencies.

Usage

In your Brocfile.js specify a dependency before calling app.toTree(). The following example scenarios should illustrate how this works.

Javascript Assets

Standard Non-AMD Asset

Provide the asset path as the first and only argument:

app.import('vendor/momentjs/moment.js');
Standard AMD Asset

Provide the asset path as the first argument, and the list of modules and exports as the second:

app.import('vendor/ic-ajax/dist/named-amd/main.js', {
  'ic-ajax': [
    'default',
    'defineFixture',
    'lookupFixture',
    'raw',
    'request',
  ]
});
Environment Specific Assets

If you need to use different assets in different environments, specify an object as the first parameter. That objects keys should be the environment name, and the values should be the asset to use in that environment.

app.import({
  development: 'vendor/ember/ember.js',
  production:  'vendor/ember/ember.prod.js'
});
Customizing a built-in Asset

This is somewhat non-standard, but suppose that you have different versions of Ember specified (using the canary builds for example). You would simply manipulate the vendor tree that is passed in to the EmberApp constructor:

var EmberApp = require('ember-cli/lib/broccoli/ember-app');
var fileMover   = require('broccoli-file-mover');

var vendorTree = fileMover('vendor', {
  files: {
    'ember-dev/ember.js': 'ember/ember.js',
    'ember-prod/ember.prod.js': 'ember/ember.prod.js'
  }
});

var app = new EmberApp({
  name: require('./package.json').name,
  trees: {
    vendor: vendorTree
  }

  getEnvJSON: require('./config/environment')
});

Styles

Static CSS

Provide the asset path as the first argument:

app.import('vendor/foundation/css/foundation.css')

All style assets added this way will be concatenated and output as /assets/vendor.css.

Dynamic Styles (SCSS, LESS, etc)

The vendor trees that are provided upon instantiation are available to your dynamic style files. Take the following example (in app/styles/app.scss):

@import "vendor/foundation/scss/normalize.scss";

@stefanpenner
Copy link
Contributor

changelog entry reminder from the changelog police

@stefanpenner
Copy link
Contributor

can you add ember-data + ic-ajax like this? (as they are "optional")

Interestingly now we can simply add ember.prod and ember.js based on ENV

@rwjblue
Copy link
Member Author

rwjblue commented May 1, 2014

@stefanpenner - CHANGELOG and ember-data + ic-ajax are now in the Brocfile.

@rwjblue
Copy link
Member Author

rwjblue commented May 2, 2014

Updated to eliminate ignoredModules in favor of keeping track of them as part of using app.import. This keeps the things that are related to a given file together (and removes the bizarre sprawl that we started to see in ember-app.js).


this.import('vendor/jquery/jquery.js');
this.import('vendor/handlebars/handlebars.js');
this.import('vendor/ember/ember.js');
Copy link
Member

Choose a reason for hiding this comment

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

@rjackson maybe could we allow here an easy way to specify my ember.js path? Besides of what @stefanpenner said of using ENV to decide if we use min or not, I would like to use canary and here the story is a little more complicated (unless it changed).

I'm updating an app to this Brocfile, and I have the following in my bower.

    "ember": "http://builds.emberjs.com/canary/ember.js",
    "ember-prod": "http://builds.emberjs.com/canary/ember.prod.js",

I'm doing something like

app.legacyFilesToAppend = without(app.legacyFilesToAppend, 'ember/ember.js'); 
if (app.env === 'production') {
  app.import('ember-prod/index.js');
} else {
  app.import('ember/index.js');
}

Which doesn't work, we require Ember to before ember-load-initializers

Copy link
Member

Choose a reason for hiding this comment

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

this.import('vendor/handlebars/handlebars.js');
this.import({
development: 'vendor/ember/ember.js',
production: 'vendor/ember/ember.prod.js'
Copy link
Member

Choose a reason for hiding this comment

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

@rjackson 👍

@rwjblue
Copy link
Member Author

rwjblue commented May 2, 2014

Updated PR description with examples of new usage/API. This can serve as a basis for docs in the gh-pages branch.

@twokul
Copy link
Contributor

twokul commented May 2, 2014

I like it

@jeprussell
Copy link

(y)

@stefanpenner
Copy link
Contributor

for reference, the gist of random ideas: https://gist.github.com/stefanpenner/3ce98d7554a463f3958f

@tonywok
Copy link
Contributor

tonywok commented May 2, 2014

Will you import css/sass/scss this way as well?

@rwjblue
Copy link
Member Author

rwjblue commented May 2, 2014

@tonywok - No, I do not think so. You would follow similar steps to the Customizing a built-in Asset section above and customize the styles tree.

@rwjblue
Copy link
Member Author

rwjblue commented May 2, 2014

Now that I think about it, maybe it does need to be the same (or a similar) API.

@tonywok
Copy link
Contributor

tonywok commented May 2, 2014

@rjackson If we imported the css assets in a similar way, I think we could get a hold of them in the preprocessCss method along with all the stuff in app/styles

Pulling down your branch and playing around :)

@stefanpenner
Copy link
Contributor

@tonywok exactly

@tonywok
Copy link
Contributor

tonywok commented May 2, 2014

@stefanpenner Although I think there may still be a problem with the preprocessCss choosing a single compiler. For example, if you are using sass, then it will pick up *.scss, but I don't think it would pick up static *.css.

What are your thoughts on importing it along with some metadata - so we know what trees to merge? /cc @rjackson

Or is there some broccoli-foo I don't know about yet?

@rwjblue
Copy link
Member Author

rwjblue commented May 2, 2014

I think this handles the basic JS scenario well, I want to get the styles straightened out too.

I need some examples that I can use for testing the CSS solution. Just a handful of common libraries and/or a bower.json config and an explanation of what the expected output is.

@stefanpenner
Copy link
Contributor

@rjackson i'll through up an example i've been working on shortly

@stefanpenner
Copy link
Contributor

also fixes: #233

@JPBetley
Copy link

JPBetley commented May 3, 2014

This looks great, @rjackson! So much cleaner.

There are a couple of issues with hardcoded values, on lines 91, 142, and 201. Those should reference the trees values.

Also, how will this.import handle a change in location of the vendor directory? The loader default option and all this.import calls refer to vendor directly, rather than through the trees variable. Would adding the this.trees.vendor prefix within the import function be a good idea?
I pulled your branch and the 'vendor/package...' for this.import has no issues and works fine. My bad.

Also, I suggest adding a trees.public property for the public folder and returning that from EmberApp.prototype.publicFolder

@rwjblue
Copy link
Member Author

rwjblue commented May 4, 2014

@JPBetley - Thanks for reviewing:

I fixed two of the references (lines 91 and 201), but the third (142) has to do with the location to use as the root when searching for a .jshintrc file (and is not referencing the built-up broccoli tree at all).

RE: public folder, I am planning on another pass through (after this lands) to make the public/ folder excluded from the standard broccoli pipeline. This would mean that the assets in public/ would be served directly through Express and do not have to be copied through the entire broccoli workflow.

@rwjblue
Copy link
Member Author

rwjblue commented May 4, 2014

Updated to handle styles better.

@rwjblue
Copy link
Member Author

rwjblue commented May 5, 2014

Updated with examples of both dynamic and static styles.

rwjblue and others added 5 commits May 5, 2014 00:31
Lets loader + wrapInEval also be configurable.

We plan soon to make loader configurable via the underlying filter.
Please note, the current setup still does not allow this.
Defaults only allows single depth merges.
@stefanpenner
Copy link
Contributor

lets get a PR ready for the website aswell, i'll try to doit tonight after which hopefully we can cut the release.

stefanpenner added a commit that referenced this pull request May 5, 2014
Add `app.import` to control legacy files, remove broccoli-bower.
@stefanpenner stefanpenner merged commit fc3f69c into ember-cli:master May 5, 2014
@rwjblue rwjblue deleted the remove-broccoli-bower branch July 3, 2014 02:00
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.

8 participants