Skip to content

refactor(init): run link-cli before npm-install #2250

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

Closed
wants to merge 2 commits into from
Closed

refactor(init): run link-cli before npm-install #2250

wants to merge 2 commits into from

Conversation

monojack
Copy link
Contributor

No description provided.

@kylecordes
Copy link

Excellent... Great improvement in first use experience, looking forward to this.

@hansl
Copy link
Contributor

hansl commented Sep 21, 2016

@filipesilva: we moved it to after in #2086 because Ember-CLI was using the wrong dependency, but was this only for our e2e tests? We later updated e2e to use node tests/e2e_runner.js instead of using npm run e2e, so maybe this is working now.

Could you check it out?

if it doesn't work, I think we should just kill the --link-cli functionality altogether.

@monojack
Copy link
Contributor Author

monojack commented Sep 21, 2016

@hansl, This was intended to reduce the time it takes to scaffold a new project by skipping re-installing the angular-cli package inside the project's node_modules. It links to the angular-cli global package and when npm install runs, it finds the symlink and skips over. Reduces the time with more than 60-70%

@Meligy
Copy link
Contributor

Meligy commented Sep 21, 2016

Except, the problem with this behavior (as documented in #2086), is that it makes everyone hit by the node_modules inside the angular-cli repo folder being deleted, due to npm/npm#10343, which is why #2086 changed it as I understand.

@aecz
Copy link
Contributor

aecz commented Sep 21, 2016

It looks like it only affects developers who have angular-cli repo locally and npm linked it.
For developers that just install angular-cli globally, it works and saves a lot of time and space.

npm i -g angular-cli
ng new myproject --link-cli

=> Global installation of angular-cli is not affected. So as @hansl said, the issue could be with angular-cli e2e tests but the build in appveyor went well for Node 5 so please keep this functionality.

And here are some metrics:

Project size:

  • without link: 218MB
  • with link: 60MB

=> 72% smaller

Command time (only ng new, angular-cli 1.0.0-beta.15 already installed globally):

  • without link: 5 min 40s
  • with link: 1 min 23 s

=> 76% faster

On Windows 7, Intel i7, node 6.5.0, npm 3.10.6

@kylecordes
Copy link

My timing ratios are similar to those from @aecz - although it sounds like there is a remaining issue here (--link-cli works with an ordinary global install of CLI, but with a local dev repo who linked that one globally? I didn't try it on the same machine with my source checkout), it seems worth fixing as the user experience is so much better. In particular it is much better for developers on Windows machines - for historical technical reasons I have forgotten, the file system implementation in Windows performs well reading and writing huge files but not so well reading and writing a large number of small files.

Is it possible that this link option could be polished sufficiently to be made the default?

@Meligy
Copy link
Contributor

Meligy commented Sep 21, 2016

A proper test I imagine would be to skip NPM installation (--sn) and try all the steps manually like:

npm rm -g angular-cli
npm cache clear
npm i -g angular-cli
ng new myproject --sn
cd myproject
npm link angular-cli
npm i

If this doesn't affect the folder of the globally installed angular-cli, then this means that npm/npm#10343 affects only developers who have angular-cli repo locally as mentioned above (which will still need to be warned in docs or something).

@filipesilva
Copy link
Contributor

filipesilva commented Sep 22, 2016

As mentioned in #2250 (comment), this change was introduced in #2086 to address npm/npm#10343.

npm/npm#10343 is very well summarized in its title:

npm install after npm link will "steal" dependencies from linked packages

It currently has the big-bug label in https://github.com/npm/npm. This means that it is a big bug and hence a big problem. For some reason or another it might not be affecting you in particular, but it's still an outstanding problem in npm3.

It's not very relevant that switching the order of npm link and npm install would make it faster if it potential breaks your install.

The --link-cli option is solely meant for working on the CLI while also testing changes on a project. The difference in install times is irrelevant for users because this is only used for development. It doesn't make any sense at all to make it the default because that completely sidesteps the point of having a local version.

If you want to use the --link-cli option for anything but CLI development, you do so at your own risk. Please don't open issues regarding alternative uses of this option.

@aecz
Copy link
Contributor

aecz commented Sep 22, 2016

I think the flag --link-cli for originally intented to link to the global version of angular-cli as stated in #778 .

For those who wants this behavior and do not have angular-cli repo linked locally. You can use these commands:

npm i -g angular-cli (unless it is already installed)
ng new --link-cli --skip-npm myproject
cd myproject
npm install

Much faster and lighter. Perfect when you make a demo of how great is angular-cli.

@filipesilva
Copy link
Contributor

@aecz it's true that initially that was the purpose of --link-cli, but as it stands at the moment there are several reasons why this is a bad idea:

  • the CLI started having a build step
  • the CLI started having subpackages
  • doing npm install after npm link can steal dependencies

Thus we cannot neither recommend nor support that usage.

@kylecordes
Copy link

Since it turns out that the NPM bug gets in the way of this efficiency/speed initial purpose, would be wise to park the link feature out of the way somehow? Here are a couple of ideas come to mind:

  • Remove --link-cli, and people developing type an extra command to link manually?
  • Rename it something like --link-for-cli-development or some other name that tries to clarify its more narrow purpose?
  • When executed, make it notice it has been linked to a global installation, and print out a warning?

Offhand it seems that one of these could save some trouble for people who make their way into this feature somehow, are enthused at the apparent initial project-setup speed improvement (dramatic on Windows) without realizing the problems.

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants