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

[ENHANCEMENT] Addon-import support for built-in blueprints #3690

Merged
merged 1 commit into from
Apr 5, 2015

Conversation

trabus
Copy link
Contributor

@trabus trabus commented Mar 28, 2015

This PR changes the way blueprints are generated inside addons, following the recommendation here of importing the actual addon code into the app with a wrapper, like so:

// addon/components/foo-bar.js
import Ember from 'ember';
export default Ember.Component.extend({
  // business logic
});

// app/components/foo-bar.js
import FooBar from 'my-addon/components/foo-bar';
export default FooBar;

I copied the component-addon blueprint with a general purpose addon-import blueprint that will allow it to be used for all blueprints that need it.

I still need to create generate and destroy tests for all the blueprints that had support added, and will do so this evening. I also need to make sure that the blueprints that shouldn't generate the import (anything that doesn't have a __root__ token and isn't a mixin) aren't doing so, or causing any other side effects. Mostly wanted to put it up for review.

@trabus trabus changed the title [WIP ENHANCEMENT] Addon-import support for built-in blueprints [WIP][ENHANCEMENT] Addon-import support for built-in blueprints Mar 28, 2015
@trabus trabus force-pushed the addon-import-blueprints branch 2 times, most recently from 17e136f to a2a701e Compare March 30, 2015 07:01
@trabus
Copy link
Contributor Author

trabus commented Mar 30, 2015

Added some tests for generating inside an addon, and added a silent error for generating addons that don't contain a __root__ token, so we can exclude blueprints that generate inside addons by simply not using the __root__ token. On of the blueprints that I've removed the token from is routes, since they can't add to route.js when generating so they throw an error, and so they can't support in-addon generation at this time.

@trabus
Copy link
Contributor Author

trabus commented Apr 3, 2015

@rwjblue @stefanpenner this is ready for review.

I wasn't quite sure which blueprints we should generate in this manner, but right now I've got route and resource throwing a warning, since they error anyway trying to modify route.js. The component and mixin blueprints already were setup, so those had no change.

I cleaned up the tests a bit, creating addon and in-repo-addon destroy and generate tests, and I moved the already existing ones out of the generate and destroy tests.

@trabus trabus changed the title [WIP][ENHANCEMENT] Addon-import support for built-in blueprints [ENHANCEMENT] Addon-import support for built-in blueprints Apr 3, 2015
@stefanpenner
Copy link
Contributor

LVGTM, but i will want to do a more thorough review and test it out in the AM (getting a bit sleepy)

@stefanpenner stefanpenner added this to the v0.2.3 milestone Apr 3, 2015
rwjblue added a commit that referenced this pull request Apr 5, 2015
[ENHANCEMENT] Addon-import support for built-in blueprints
@rwjblue rwjblue merged commit 2e64faa into ember-cli:master Apr 5, 2015
@rwjblue
Copy link
Member

rwjblue commented Apr 5, 2015

PERFECT THANK YOU!!!

@trabus trabus deleted the addon-import-blueprints branch April 5, 2015 18:53
@jonathanKingston
Copy link
Member

@trabus I needed this so much last week, however I am sure I will need it again and again 👍 thanks.

jayphelps added a commit to jayphelps/ember-cli that referenced this pull request Apr 8, 2015
jayphelps added a commit to jayphelps/ember-cli that referenced this pull request Apr 8, 2015
stefanpenner added a commit that referenced this pull request Apr 12, 2015
Use shorthand ES6 re-export for addon-imports as well, which landed in #3690
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

5 participants