feat(install): 1. and 3. phase from design docs by jkuri · Pull Request #294 · angular/angular-cli

jkuri · 2016-03-11T20:43:17Z

Install example: ng install moment --typings moment-node
Uninstall examle: ng uninstall lodash --typings lodash

cironunes · 2016-03-12T14:57:56Z

no tests? :)

jkuri · 2016-03-12T14:59:15Z

Not yet :-)

hansl · 2016-03-13T03:59:51Z

addon/ng2/tasks/uninstall.js

+  uninstallProcedure: function() {
+    const that = this;
+
+    return that.removeFromPackageJSON(that.package)


The goal of promises was to avoid this kind of thing :) you can simplify this like this:

return that.removeFromPackageJSON(that.package) .then(() => that.npmUninstall(that.package)) .then(() => that.unlinkPackage(that.package)) .then(() => that.removeFromSystemJSConfig(that.package)) .then(() => { that.ui.stopProgress(); if (that.typings) { that.ui.startProgress(chalk.green('Uninstalling typings:', chalk.yellow(that.typings)), chalk.green('.')); return that.typingsUninstall(that.typings) } }) .then(() => { that.ui.stopProgress(); that.announceOKCompletion(); });

This uses both the fact that the arrow function returns the value if you don't put it in a block, and that a promise executor returns a promise.

thanks Ciro.

hansl · 2016-03-17T23:03:26Z

addon/ng2/tasks/uninstall.js

+    this.ui.writeLine(chalk.red(this.completionErrorMessage));
+  },
+
+  existsSync: function(path) {


This seems unused.

hansl · 2016-03-17T23:42:25Z

addon/ng2/blueprints/ng2/files/src/config/system.config.js

@@ -0,0 +1,8 @@
+var systemConfig = {


Could you use System.config(...) here directly?

Sure, its nicer. Done.

hansl · 2016-03-31T22:02:44Z

.eslintignore

@@ -0,0 +1 @@
+addon/ng2/blueprints/ng2/files/src/config/system.config.js


Why do we want to ignore that single file? It seems like it should be linted.

Because of the double quotes which are mandatory for JSON.parse in systemjs-helper.ts.

hansl · 2016-04-01T03:19:08Z

addon/ng2/tasks/install.ts

+    return new Promise(resolve => {
+      let promises = [];
+
+      that.packages.forEach(p => {


This sounds more like a map() than a forEach().

hansl · 2016-04-01T18:01:33Z

addon/ng2/tasks/uninstall.ts

+    this.packages = this.packages.filter(p => !/node-sass|stylus|less|compass-importer/.test(p));
+    let typingsList = [];
+
+    return new Promise(resolve => {


You don't need that, just return Promise.all(...).then(...). then() returns a Promise.

Thanks, updated.

hansl · 2016-04-01T18:31:42Z

lib/broccoli/angular-broccoli-less.js

+    }
+
+    compile(fileName, inputPath, outputPath) {
+      return new Promise(resolve => {


You can do something like this to simplify:

compile(fileName, inputPath, outputPath) { let content = fs.readFileSync(fileName, 'utf8'); return less.render(content) .then(output => { let filePath = fileName.replace(inputPath, outputPath).replace(/\.less$/, '.css'); fse.outputFileSync(filePath, output.css, 'utf8'); }); }

please do that for all compile() functions :)

jkuri · 2016-04-01T20:20:42Z

Closing this. If nothing else, I learned something from this PR :-) Thanks @hansl!

choeller · 2016-04-04T17:30:15Z

@jkuri Do I understand it correct, that this did not make it into the master? And if so - why?! 😳 I think this a super essential feature and many people are looking forward to it...

jkuri · 2016-04-04T17:38:31Z

We had some problems with typings. This will be implemented but cannot tell you when because I don't know it myself :) If you are interested for sass/less support you can follow it here, the rest of the things from design docs will follow later.

choeller · 2016-04-04T17:40:46Z

Oh, ok - that's sad to hear :/ so to use 3rd party libraries like ngrx we still need to copy the sources to the project?

jkuri · 2016-04-04T17:41:29Z

Unfortunately yes

hansl · 2016-04-04T18:35:37Z

I just want to clarify that you don't need to copy files to your project
for ngrx. You should simply be able to npm install ngrx and add those
files to your vendorNpmFiles.

We are working on the production build story to use CDN versions of those
vendor files.

On Mon, Apr 4, 2016 at 10:41 AM, Jan Kuri notifications@github.com wrote:

Unfortunately yes

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#294 (comment)

angular-automatic-lock-bot · 2019-09-09T22:42:00Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

jkuri added feature Label used to distinguish feature request from other issues pr_action: review labels Mar 11, 2016

jkuri force-pushed the install branch 3 times, most recently from 3c8b2c6 to 65ac052 Compare March 12, 2016 01:49

cironunes self-assigned this Mar 12, 2016

hansl reviewed Mar 13, 2016
View reviewed changes

jkuri mentioned this pull request Mar 13, 2016

fix(serve): rebuilds after new typings are installed #295

Closed

cironunes removed their assignment Mar 13, 2016

jkuri force-pushed the install branch 5 times, most recently from 2768925 to 2b01261 Compare March 17, 2016 22:55

hansl reviewed Mar 17, 2016
View reviewed changes

jkuri force-pushed the install branch from 2b01261 to bbaaaf8 Compare March 17, 2016 23:16

hansl reviewed Mar 17, 2016
View reviewed changes

jkuri added 2 commits March 31, 2016 23:23

feat(build): broccoli stylus plugin

33833b8

feat(build): broccoli compass plugin

de3c89b

jkuri force-pushed the install branch from cf90640 to 79fb096 Compare March 31, 2016 21:25

hansl reviewed Mar 31, 2016
View reviewed changes

jkuri force-pushed the install branch 2 times, most recently from cf22ff9 to 7318c0d Compare April 1, 2016 03:16

hansl reviewed Apr 1, 2016
View reviewed changes

jkuri force-pushed the install branch 4 times, most recently from 387b3d5 to 319cb3a Compare April 1, 2016 17:56

hansl reviewed Apr 1, 2016
View reviewed changes

jkuri force-pushed the install branch from 319cb3a to e87cdfd Compare April 1, 2016 18:08

hansl reviewed Apr 1, 2016
View reviewed changes

chore(test): e2e css preprocessor tests

d00177c

jkuri force-pushed the install branch from e87cdfd to d00177c Compare April 1, 2016 19:54

jkuri closed this Apr 1, 2016

jkuri deleted the install branch April 3, 2016 00:06

filipesilva mentioned this pull request Apr 6, 2016

ng install does not support @ versions #356

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019

		@@ -0,0 +1 @@
		addon/ng2/blueprints/ng2/files/src/config/system.config.js

Conversation

jkuri commented Mar 11, 2016

Uh oh!

cironunes commented Mar 12, 2016

Uh oh!

jkuri commented Mar 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkuri commented Apr 1, 2016

Uh oh!

choeller commented Apr 4, 2016

Uh oh!

jkuri commented Apr 4, 2016

Uh oh!

choeller commented Apr 4, 2016

Uh oh!

jkuri commented Apr 4, 2016

Uh oh!

hansl commented Apr 4, 2016

Uh oh!

angular-automatic-lock-bot bot commented Sep 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants