(feat) 3rd party libs

[jkuri]

I got this working, but like the whole project this is a prototype and a lot of things needed to be done.
This is how it looks like

[tp]$ ng libs install ng2-cli-test-lib
version: 1.13.13
Installing 3rd party packages: ng2-cli-test-lib...
1 Packages successfully installed
[?] Would you like to inject the installed packages into your app automatically? Y
[?] What is the path to the file which bootstraps your app? /Users/jan/Desktop/tp/src/app.ts

[?] What would you like to inject from ng2-cli-test-lib?
1 Directive
2 Pipe
3 Provider
4 styleUrl
Enter value: 1
[?] Which Directive would you like to inject?
1 TestDirective
Enter value: 1
[?] Where would you like to inject it?
1 /Users/jan/Desktop/tp/src/app.ts
2 /Users/jan/Desktop/tp/src/app/tp.spec.ts
3 /Users/jan/Desktop/tp/src/app/tp.ts
Enter value: 3
Injecting Directive (TestDirective) to /Users/jan/Desktop/tp/src/app/tp.ts
Done.

[tp]$

I tested on a https://github.com/NathanWalker/ng2-cli-test-lib test library, which regarding to comments on #96 should be it.

[NathanWalker]

@jkuri you are amazing!
Really nice work thank you!
We may tweak the flow a little bit but this is definitely what we are after. Bravo 👍

[jkuri]

@NathanWalker no problem. Would you write some guide how the packages needs to be created?

[NathanWalker]

@jkuri You nailed it for most part.
I will write a guide once we have consensus. I'll cc' others below that may want to offer other ideas.

Main thing I object to is:

What would you like to inject from ng2-cli-test-lib?

In general, I believe it should ask something like this before that:

Would you like to customize the injection of ng2-cli-test-lib? (N/y)

If n (default):

Please provide the path to the file which bootstraps your root component:

/path/to/bootstrap.ts

// cli appends all lib providers to bootstrap.

Please provide the path to the component where you want the (directives, pipes, styles, styleUrls, etc.) added?

/path/to/root/component.ts

// cli injects and annotates everything else that the lib makes available.

If 'y':
Then it would go through the flow you have at the moment, which is more pick/choose (manual install) from the library.

I believe the default should be the easiest most painless path, which would be:
Add the lib, inject and annotate with everything the lib provides and don't allow the user to pick/choose

Path of least resistance I believe is adding providers to the apps bootstrap method ensuring it's available app-wide. This will help devs out of the gate I believe. More advanced users can answer y to Would you like to customize the injection of ng2-cli-test-lib? (N/y) to go the pick/choose (more power) route.

/cc @jvandemo, @gdi2290, @valorkin, @ocombe, @robwormald

[jkuri]

My last commit allows auto import (this is default option) of everything that lib provide in bootstrap script. It also injects providers in bootstrap function.

[NathanWalker]

@jkuri this is exciting! I can't thank you enough, I can't wait for this to land....

@IgorMinar please, pretty please? :)

lgtm. It can be improved over time if need be and the flow can continue to be tweaked, but having this now will allow lib authors to start advocating angular cli as a way to install their lib 👍

[jkuri]

@IgorMinar yes, that would be cool :-). This command/task does not require any new dependencies, thanks to inquirer is built in ember-cli.

[filipesilva]

Hey @jkuri, could you rebase this on master?

[valorkin]

need to check my modules to work with this

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[jkuri]

Hi @filipesilva, I rebased this.

[googlebot]

CLAs look good, thanks!

[jkuri]

@valorkin use this script with systemjs-builder and run it in prepublish script. Also, here is example how to export things.

[hansl]

@jkuri: Hey Jan, good job!

1. any reason why ng libs install foo was picked over ng install foo? I personally prefer smaller command lines. Seems like install could be purposed for this. I'm going to comment on Proposal: Auto add (providers, directives, pipes, styles, styleUrls) when adding 3rd party libs, etc. #96 about this.
2. please squash your history before merging. We want to keep a clean commit history (same code practice as other angular projects). If you need help, you can check here and/or ask me!

The code looks good but I'm going to read it more thoroughly later on today.

[jkuri]

Hi @hansl. Thank you!

Yes, I do need some help with squashing commits into one.. here's what I am trying to do
git rebase -i HEAD~9
then I mark all but first one with s and the first one I left with pick option. Then it rebases my commits, but when trying to push to a 3rd-party-libs branch it says my current branch is behind. Any suggestions?

I also like shorter commands but in that case I choose ng libs for two reasons. First is that when you type libs you know that you will install 3rd party library, and the second is that correlated install and uninstall commands can be merged into one ember-cli task.

[NathanWalker]

@jkuri Whenever you squash commits, you are rewriting history ;)
So you have to force push:

git push -f

[hansl]

Unfortunately, you need to force push at this point.

[jkuri]

Thanks. Commits squashed.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

[googlebot]

CLAs look good, thanks!

[cironunes]

This is pretty cool! Thanks @jkuri.

I'd do 2 things before merging this in:

• Change the command to ng lib install (as we are installing one lib per time, I think that keeping it in singular make more sense) or ng install as suggested by @hansl
• Add unit tests for the feature

[jkuri]

@cironunes thanks, no problem.
You can install/uninstall multiple libs per time, separating them by space.
ng install and ng uninstall as @hansl suggested will be good, but then I need to break up the task into two separated tasks. Also ng i and ng u will then be possible as aliases to this commands, even shorter. Should I implement it in that way?

[hansl]

@jkuri: I personally don't see it as a blocker for this PR. It can easily be done in a separate PR. I checked and the ember install currently fails so people won't be using it.

You're 👍 to commit.

[jkuri]

@hansl ok, I agree. I can override the core install command, but if I remember correctly it logs some warning that the command has been overridden. Will check if I can take care of that.

[NathanWalker]

@jkuri not a huge deal, but you might reword your commit msg from:

(feat) 3rd party libs

to:

feat(libs): 3rd party lib helper

closes https://github.com/angular/angular-cli/issues/96

[hansl]

And please squash your history. I can take time right off now to help you merge if you need to.

[jkuri]

Thanks, I managed to squash it yesterday, I will make another commit when separating commands later today. I was having problem with squashing merged stuff from master. It will not harm if you write commands in order, it can save me some time.

[hansl]

I would suggest doing a separate PR just so we can get this in. Also, I feel this second commit came from a git merge; you should rather do git rebase master instead (assuming your master is up to date). It doesn't commit anything (although you have to git push --force)

[NathanWalker]

git checkout master
git fetch upstream
git rebase upstream/master
git checkout 3rd-party-libs
git rebase master  // if you have merge conflicts, you may need to fix and add, then `git rebase --continue`
git rebase -i HEAD~4  // reword, squash and do what you need to do
git push -f

[jkuri]

@hansl and @NathanWalker thanks for your help with that. I squashed commits without any problems now.

I reword commands to ng install <name-of-lib> and ng uninstall <name-of-lib> and put them in separate tasks. Here is one variant of how install process looks like, and here is screenshot of uninstall process. All the things gets properly injected and also cleaned after uninstalling, but I still need to write unit tests.
After this will be merged, we should write documentation about how the lib needs to be created to be compatible with our commands, so developers can start building/reconfigures their libs the right way.

[NathanWalker]

Agreed, I will help write a lib guide over next 2 weeks (prob become its
own repo with wiki and all) that we can collab on to make good docs for how
to author libs that will work with this feature. Again, really nice work
here.

On Sun, Jan 24, 2016 at 5:43 PM Jan Kuri [email protected] wrote:

@hansl https://github.com/hansl and @NathanWalker
https://github.com/NathanWalker thanks for your help with that. I
squashed commits without any problems now.

I reword commands to ng install and ng uninstall
and put them in separate tasks. Here
https://cloud.githubusercontent.com/assets/1796022/12540605/bfb40d96-c30b-11e5-8397-5f0ca544c417.png
is one variant of how install process looks like, and here
https://cloud.githubusercontent.com/assets/1796022/12540653/8133b142-c30c-11e5-8e53-d009620bd027.png
is screenshot of uninstall process. All the things gets properly injected
and also cleaned after uninstalling, but I still need to write unit tests.
After this will be merged, we should write documentation about how the lib
needs to be created to be compatible with our commands, so developers can
start building/reconfigures their libs the right way.

—
Reply to this email directly or view it on GitHub
#135 (comment).

[hansl]

If we do documentation I suggest keeping it under a single roof. I don't mind either creating a doc/ folder with markdown files or using this github's wiki.

[NathanWalker]

Starting with the wiki here sounds like a good idea.

On Sun, Jan 24, 2016 at 5:53 PM Hans [email protected] wrote:

If we do documentation I suggest keeping it under a single roof. I don't
mind either creating a doc/ folder with markdown files or using this
github's wiki.

—
Reply to this email directly or view it on GitHub
#135 (comment).

[jkuri]

Added tests to both commands.

[cironunes]

There you go. Awesome job @jkuri. LGTM

[filipesilva]

Awesomesauce. Merging it then!

[jkuri]

Thanks for helping guys.

[NathanWalker]

Thank you to everyone here. @jkuri incredible effort. If you are on Twitter, you might DM me, we can chat about further details of documentation coordination, etc (@wwwalkerrun)

[ocombe]

\o/

Right on time for my talk at NG-NL on "How to write libraries for Angular 2" :)

[filipesilva]

Sweet. Tells us how it goes on gitter! https://gitter.im/angular/angular-cli

[cironunes]

can't wait for the next release :)

[angular-automatic-lock-bot]

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.}