[BUGFIX] `ember init` fails on `NULL_PROJECT` #546

twokul · 2014-04-30T18:22:00Z

Previously, if you try to run ember init inside an empty directory, it will
fail because environment.project was not set to NULL_PROJECT.

Steps:

mkdir foo && cd foo && ember init

thanks to @abuiles for initial spike, #529

Closes #540

twokul · 2014-04-30T18:24:28Z

/cc @stefanpenner @MajorBreakfast @rjackson @abuiles

MajorBreakfast · 2014-04-30T18:26:32Z

lib/commands/init.js

@@ -23,7 +23,7 @@ module.exports = new Command({
    var installBlueprint = environment.tasks.installBlueprint;
    var npmInstall       = environment.tasks.npmInstall;
    var bowerInstall     = environment.tasks.bowerInstall;
-    var packageName      = environment.project ? environment.project.pkg.name : path.basename(cwd);
+    var packageName      = environment.project.isEmberCLIProject() ? environment.project.pkg.name : path.basename(cwd);


Make project a var, I'm not a fan of lines that long

var project = environment.project;

abuiles · 2014-04-30T18:28:38Z

@twokul I did this at the beginning but it was breaking ember new can you please verify that is still working? ember new foo

twokul · 2014-04-30T18:41:19Z

@abuiles it works.

abuiles · 2014-04-30T18:44:51Z

@twokul 👍 thanks!

stefanpenner · 2014-04-30T18:54:10Z

lib/commands/init.js

@@ -23,7 +23,8 @@ module.exports = new Command({
    var installBlueprint = environment.tasks.installBlueprint;
    var npmInstall       = environment.tasks.npmInstall;
    var bowerInstall     = environment.tasks.bowerInstall;
-    var packageName      = environment.project ? environment.project.pkg.name : path.basename(cwd);
+    var project          = environment.project;
+    var packageName      = project.isEmberCLIProject() ? environment.project.pkg.name : path.basename(cwd);


i would prefer if the null project return a null package name, as the null pattern is to reduce such branching.

project.isEmberCLIProject() ? environment.project.pkg.name : path.basename(cwd);
-> project.isEmberCLIProject() ? project.pkg.name : path.basename(cwd);

@stefanpenner I'm not so sure if we should extend the NULL_PROJECT to include these things. I even think that the pkg === undefined is the desired thing because the file really doesn't exist.

I can change it to something like:

var packageName = project.name(); // ... if (!packageName) { packageName = path.basename(cwd); }

That will reduce branching.

but I'm generally ok with

project.isEmberCLIProject() ? environment.project.pkg.name : path.basename(cwd);

kinda seems like if no name exists, the NULL_PROJECT's name should be set to basename of root, and root should be the cwd

No need to clutter Project with this special case. It's already a one-liner.

@MajorBreakfast i think that is the point of this pattern

stefanpenner · 2014-04-30T20:52:50Z

LGTM

abuiles · 2014-04-30T21:37:14Z

tests/unit/commands/init-test.js

+        name: function() {
+          return 'test';
+        },
+        isEmberCLIProject: function() {


probably isEmberCLIProject is not longer required here ?

stefanpenner · 2014-04-30T23:11:41Z

lib/models/project.js

@@ -33,6 +33,10 @@ NULL_PROJECT.isEmberCLIProject = function() {
  return false;
 };

+Project.prototype.name = function() {
+  return path.basename(process.cwd());


this is not correct, this is the behavior only of the NULL_PROJECT, otherwise name should come form this.pkg.name

@abuiles

Previously, if you try to run `ember init` inside an empty directory, it will fail because `environment.project` was not set to `NULL_PROJECT`. thanks to @abuiles for initial spike, [#529](#529) Steps: ```bash mkdir foo && cd foo && ember init ``` Closes #540

abuiles · 2014-05-01T13:02:57Z

@twokul LGTM @rjackson @stefanpenner @MajorBreakfast mind giving a last look before merging .

twokul · 2014-05-01T13:03:46Z

@abuiles thanks for the feedback

[BUGFIX] `ember init` fails on `NULL_PROJECT`

twokul mentioned this pull request Apr 30, 2014

Check pkg before trying to read name. #529

Closed

MajorBreakfast reviewed Apr 30, 2014
View reviewed changes

stefanpenner reviewed Apr 30, 2014
View reviewed changes

abuiles reviewed Apr 30, 2014
View reviewed changes

zigomir mentioned this pull request Apr 30, 2014

handle ember init inside directory without package.json #550

Closed

stefanpenner reviewed Apr 30, 2014
View reviewed changes

rwjblue added a commit that referenced this pull request May 1, 2014

Merge pull request #546 from twokul/init-fix

bb2f49f

[BUGFIX] `ember init` fails on `NULL_PROJECT`

rwjblue merged commit bb2f49f into ember-cli:master May 1, 2014

twokul deleted the init-fix branch May 1, 2014 13:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUGFIX] `ember init` fails on `NULL_PROJECT` #546

[BUGFIX] `ember init` fails on `NULL_PROJECT` #546

twokul commented Apr 30, 2014

twokul commented Apr 30, 2014

MajorBreakfast Apr 30, 2014

twokul Apr 30, 2014

abuiles commented Apr 30, 2014

twokul commented Apr 30, 2014

abuiles commented Apr 30, 2014

stefanpenner Apr 30, 2014

MajorBreakfast Apr 30, 2014

twokul Apr 30, 2014

twokul Apr 30, 2014

stefanpenner Apr 30, 2014

MajorBreakfast Apr 30, 2014

stefanpenner Apr 30, 2014

stefanpenner commented Apr 30, 2014

abuiles Apr 30, 2014

stefanpenner Apr 30, 2014

abuiles commented May 1, 2014

twokul commented May 1, 2014

[BUGFIX] ember init fails on NULL_PROJECT #546

[BUGFIX] ember init fails on NULL_PROJECT #546

Conversation

twokul commented Apr 30, 2014

twokul commented Apr 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abuiles commented Apr 30, 2014

twokul commented Apr 30, 2014

abuiles commented Apr 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanpenner commented Apr 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abuiles commented May 1, 2014

twokul commented May 1, 2014

[BUGFIX] `ember init` fails on `NULL_PROJECT` #546

[BUGFIX] `ember init` fails on `NULL_PROJECT` #546