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

Import blueprints from ember-cli #13029

Merged
merged 23 commits into from
Mar 15, 2016
Merged

Import blueprints from ember-cli #13029

merged 23 commits into from
Mar 15, 2016

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Feb 29, 2016

This PR imports the blueprints and blueprint acceptance tests from ember-cli and continues #13000:

  • acceptance-test - Generates an acceptance test for a feature
  • component - Generates a component
  • component-addon - Generates a component
  • component-test - Generates a component integration or unit test
  • controller - Generates a controller
  • controller-test - Generates a controller unit test
  • helper - Generates a helper function
  • helper-addon - Generates an import wrapper
  • helper-test - Generates a helper unit test
  • initializer - Generates an initializer
  • initializer-addon - Generates an import wrapper
  • initializer-test - Generates an initializer unit test
  • instance-initializer - Generates an instance initializer
  • instance-initializer-addon - Generates an import wrapper
  • instance-initializer-test - Generates an instance initializer unit test
  • mixin - Generates a mixin
  • mixin-test - Generates a mixin unit test
  • route - Generates a route and a template, and registers the route with the router
  • route-addon - Generates import wrappers for a route and its template
  • route-test - Generates a route unit test
  • service - Generates a service
  • service-test - Generates a service unit test
  • template - Generates a template
  • test-helper - Generates a test helper
  • util - Generates a simple utility module/function
  • util-test - Generates a util unit test

Blueprints that should remain in ember-cli:

  • addon - The default blueprint for ember-cli addons
  • addon-import - Generates an import wrapper
  • app - The default blueprint for ember-cli projects
  • blueprint - Generates a blueprint and definition
  • http-mock - Generates a mock api endpoint in /api prefix
  • http-proxy - Generates a relative proxy to another server
  • in-repo-addon - The blueprint for addon in repo ember-cli addons
  • lib - Generates a lib directory for in-repo addons
  • resource - Generates a model, template, route, and registers the route with the router
  • server - Generates a server directory for mocks and proxies
  • vendor-shim - Generates an ES6 module shim for global libraries

/cc @stefanpenner

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 2, 2016

@stefanpenner could you please verify the list of blueprints to import? I'm not entirely sure about the blueprints on the second list.

@stefanpenner
Copy link
Member

view - Generates a view subclass
view-test - Generates a view unit test

we don't need to include these, as views are deprecated. Otherwise this list looks good.

@rwjblue / @trabus you both may want to give the list a quick scan as well.

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 3, 2016

@stefanpenner it seems that helper-addon, initializer-addon and instance-initializer-addon depend on the addon-import blueprint which is used via require() directly. I still think that addon-import should remain in ember-cli but this obviously leads to problems requiring it. Should I just use require('ember-cli/blueprints/addon-import');?

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 3, 2016

also resource seems to be kinda hard to port because it depends on route and model who are now living in separate projects

@trabus
Copy link

trabus commented Mar 3, 2016

I think resource can safely go away. It's not technically supported anymore.

@trabus
Copy link

trabus commented Mar 3, 2016

@Turbo87 Regarding the addon-import stuff, yes, I would import directly from ember-cli as you were asking. Semi-related to addon-import, I'd like to maybe take an opportunity to rename those blueprints to be more specific: ember-cli/ember-cli#5402

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 4, 2016

I would import directly from ember-cli

ember-cli is not a dependency of ember. I'm not sure if that creates problems.

@homu
Copy link
Contributor

homu commented Mar 4, 2016

☔ The latest upstream changes (presumably #13048) made this pull request unmergeable. Please resolve the merge conflicts.

@Turbo87 Turbo87 force-pushed the blueprints branch 2 times, most recently from 1eb9e99 to 6859e49 Compare March 4, 2016 12:59
@Turbo87
Copy link
Member Author

Turbo87 commented Mar 4, 2016

porting route-addon is blocked on resolving ember-cli/ember-cli#5574

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 4, 2016

solved this similar to ember-cli/ember-cli#5575

this PR should be ready for review and potentially merging now

/cc @rwjblue @stefanpenner @mmun @trabus

@Turbo87 Turbo87 changed the title [WIP] Import blueprints from ember-cli Import blueprints from ember-cli Mar 4, 2016
@stefanpenner
Copy link
Member

LVGTM, I plan to try test this out today.

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 4, 2016

FYI not yet compatible with ember-cli-mocha, but I'll work on that once this PR is merged

@trabus
Copy link

trabus commented Mar 4, 2016

@Turbo87 it might be worth copying the addon-import blueprint to ember itself, and using that instead of importing the one from ember-cli.

@@ -0,0 +1,3 @@
/*jshint node:true*/

module.exports = require('ember-cli/blueprints/addon-import');
Copy link
Member

Choose a reason for hiding this comment

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

ya, we should likely extract addon-import. Don't want ember bringing ember-cli with.

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, I'm not sure if that's so easy. addon-import is used by default in ember-cli so we probably shouldn't remove it there but I'm not a big fan of duplication either.

then again since it's the default there I'm not sure why this helper-addon blueprint is even needed if it just passes on to the default. the only advantage would be to be able to generate it directly with ember g helper-addon.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should try to decouple. ember bundling ember-cli is no-beuno.

I admittedly though haven't investigated this deeply today, it is possible im missing something

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should try to decouple. ember bundling ember-cli is no-beuno.

totally agree on that 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

solved it like this: a12a193

@homu
Copy link
Contributor

homu commented Mar 6, 2016

☔ The latest upstream changes (presumably #13057) made this pull request unmergeable. Please resolve the merge conflicts.


var fs = require('fs-extra');
var path = require('path');
var Promise = require('ember-cli/lib/ext/promise');
Copy link
Member

Choose a reason for hiding this comment

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

^^

Copy link
Member Author

Choose a reason for hiding this comment

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

since ember-cli is a dev dependency on ember this should be okay, but I'm open to better suggestions.

Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 9, 2016
Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 9, 2016
Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 9, 2016
@homu
Copy link
Contributor

homu commented Mar 10, 2016

☔ The latest upstream changes (presumably #13067) made this pull request unmergeable. Please resolve the merge conflicts.

@homu
Copy link
Contributor

homu commented Mar 12, 2016

☔ The latest upstream changes (presumably #13087) made this pull request unmergeable. Please resolve the merge conflicts.

Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 14, 2016
@stefanpenner
Copy link
Member

@emberjs/core we are basically ready to merge this, and this is one of the steps to fix version issues between ember-cli and ember (and get us off bower) r?

"route-recognizer": "0.1.5",
"rsvp": "~3.1.0",
"serve-static": "^1.10.0",
"simple-dom": "^0.2.7",
"testem": "^1.0.0-rc.3"
},
"dependencies": {
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Seems fine. FWIW tooling like webpack expects npm dependencies to be things you expect to bring into the client-side with you. Taking this step of adding deps will limit our ability to ship Ember itself in a webpack-compatible manner.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is something that would need to get solved on the ember-cli side more globally. If we put those dependencies into devDependencies instead, they usually won't get installed and might be missing when someone tries to generate blueprints.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The two systems (build-time deps for Ember addons using dependencies, and runtime deps for webpack using dependencies) are at odds. That's what I'm pointing out. I totally acknowledge that you need to add these here if Ember itself is an addon.

We can say "We don't care about webpack users" or "they can use old-school vendored assets", but I'd like to see some feedback on that approach.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately that world view isn't compatible with ours. Our add ons bring there build-debs along, so they can be built by consumers.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @stefanpenner, their worldview just isn't compatible and I don't want to burn cycles worrying about people trying to use webpack.

@mixonic
Copy link
Sponsor Member

mixonic commented Mar 14, 2016

I think test-helper can be checked off on the list :-)

@mmun
Copy link
Member

mmun commented Mar 14, 2016

Thank you so much @Turbo87 !

@stefanpenner
Copy link
Member

@emberjs/core seems like generally positive about this, are we ok with merging? Or what steps are required.

rwjblue added a commit that referenced this pull request Mar 15, 2016
Import blueprints from `ember-cli`
@rwjblue rwjblue merged commit 9c9c045 into emberjs:master Mar 15, 2016
@homu homu mentioned this pull request Mar 15, 2016
8 tasks
@Turbo87
Copy link
Member Author

Turbo87 commented Mar 15, 2016

🎉

@Turbo87 Turbo87 deleted the blueprints branch March 15, 2016 06:37
Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 15, 2016
Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 15, 2016
Turbo87 added a commit to Turbo87/ember-cli-legacy-blueprints that referenced this pull request Mar 15, 2016
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