build: switch from npm to yarn #19328

ocombe · 2017-09-22T08:21:31Z

PR Type

What kind of change does this PR introduce?

[x] Build related changes
[x] CI related changes

What is the current behavior?

We use npm to install and lock our dependencies (with shrinkwrap)

What is the new behavior?

We use yarn instead of npm

Does this PR introduce a breaking change?

[x] No

mary-poppins · 2017-09-22T10:29:20Z

You can preview c706fc2 at https://pr19328-c706fc2.ngbuilds.io/.

mary-poppins · 2017-09-22T16:09:40Z

You can preview 53f4f45 at https://pr19328-53f4f45.ngbuilds.io/.

4kochi · 2017-09-22T16:26:12Z

Just out of curiosity. Why not just use npm v5? It should be almost as fast as yarn. Or is there another reason besides speed improvements?

ocombe · 2017-09-22T16:39:48Z

We still need to support old versions of node for some tests, which I believe that the new npm doesn't.
Also we will try to use yarn workspaces after this to share the node folder across all packages.

And yarn is still faster for now, see https://medium.com/@nikjohn/npm-5-yarn-killer-ba69737b24d0

4kochi · 2017-09-22T16:52:56Z

Ok, thanks for the clarification.

mary-poppins · 2017-09-22T17:02:05Z

You can preview 9c1a8d5 at https://pr19328-9c1a8d5.ngbuilds.io/.

IgorMinar · 2017-09-22T16:48:31Z

docs/DEVELOPER.md

@@ -56,7 +58,7 @@ Next, install the JavaScript modules needed to build and test Angular:

 ```shell
 # Install Angular project dependencies (package.json)
-npm install
+yarn install
 ```

 **Optional**: In this document, we make use of project local `npm` package scripts and binaries


can you also remove Options 1, 2 and 3? they all refer to installing global dependencies, using npm with yarn projects, and using sudo which are all anti-patterns and shouldn't be used.

IgorMinar · 2017-09-22T16:50:25Z

package.json

@@ -17,7 +17,8 @@
    "url": "https://github.com/angular/angular.git"
  },
  "scripts": {
-    "postinstall": "node tools/npm/copy-npm-shrinkwrap && webdriver-manager update",
+    "preinstall": "node -e \"if(process.env.npm_execpath.indexOf('yarn') === -1) throw new Error('Please use Yarn instead of NPM to install dependencies. See: https://yarnpkg.com/lang/en/docs/install/')\"",
+    "postinstall": "webdriver-manager update",


can you add the --gecko false flag here?

I wasn't sure if it would fail something with this here, I'll add it

IgorMinar · 2017-09-22T17:27:52Z

packages/common/src/pipes/date_pipe.ts

+       * If we only replace '-' with ',' in the ISO String ("2015,01,01"), and try to create a new
+       * date, some browsers (e.g. IE 9) will throw an invalid Date error
+       * If we leave the '-' ("2015-01-01") and try to create a new Date("2015-01-01") the
+       * timeoffset is applied Note: ISO months are 0 for January, 1 for February, ...


bad line join

IgorMinar · 2017-09-22T17:34:36Z

yarn.lock.readme.md

+or `yarn upgrade <packagename> --dev` to update to the latest version that matches version constraint
+in `package.json`
+
+To Remove an existing dependency do the following: run `yarn remove <packagename>@<version|latest>`


I don't think you need to specify the version in this case

IgorMinar · 2017-09-22T17:59:10Z

scripts/ci/env.sh

@@ -37,7 +37,7 @@ fi
 setEnvVar NODE_VERSION 6.9.5
 setEnvVar NPM_VERSION 3.10.7 # do not upgrade to >3.10.8 unless https://github.com/npm/npm/issues/14042 is resolved


remove this line

mary-poppins · 2017-09-22T19:01:49Z

You can preview 4976e0b at https://pr19328-4976e0b.ngbuilds.io/.

alexeagle · 2017-09-22T20:09:16Z

fyi on error-reporting: I had node 4 and the yarn install fails with

yarn install v1.0.2
$ node -e "if(process.env.npm_execpath.indexOf('yarn') === -1) throw new Error('Please use Yarn instead of NPM to install dependencies. See: https://yarnpkg.com/lang/en/docs/install/')"
error Couldn't find the binary node -e "if(process.env.npm_execpath.indexOf('yarn') === -1) throw new Error('Please use Yarn instead of NPM to install dependencies. See: https://yarnpkg.com/lang/en/docs/install/')"

not sure if it's worth fixing (by checking node version first I suppose)

ocombe · 2017-09-22T20:13:41Z

The node engine in package.json is "node": ">=6.9.5 <7.0.0",, so we probably don't need to support node 4?

alexeagle · 2017-09-22T20:17:13Z

Right, not asking for support, I'm just pointing out the error isn't very nice.
Igor is making fun of me for running node 4 at all; I don't care if you change anything for that case.

PR Close #19328

gkalpak · 2017-09-22T21:34:16Z

packages/platform-server/integrationtest/run_tests.sh

@@ -8,6 +8,6 @@ echo "#################################"
 echo "Running platform-server end to end tests"
 echo "#################################"

-npm install
+yarn install

 npm run test


Not yarn test? 😞 😛

good catch, it'll be in my follow up PR, thanks

PR Close angular#19328

angular-automatic-lock-bot · 2019-09-12T17:21:34Z

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

ocombe added area: build & ci Related the build and CI infrastructure of the project state: WIP PR target: TBD labels Sep 22, 2017

ocombe requested a review from IgorMinar September 22, 2017 08:21

googlebot added the cla: yes label Sep 22, 2017

ocombe force-pushed the build-yarn branch 2 times, most recently from 4a493a5 to 2ffe9d9 Compare September 22, 2017 09:35

angular deleted a comment from mary-poppins Sep 22, 2017

ocombe force-pushed the build-yarn branch from 2ffe9d9 to c706fc2 Compare September 22, 2017 10:17

angular deleted a comment from mary-poppins Sep 22, 2017

ocombe force-pushed the build-yarn branch from c706fc2 to 53f4f45 Compare September 22, 2017 15:57

ocombe force-pushed the build-yarn branch from 53f4f45 to 9c1a8d5 Compare September 22, 2017 16:46

ocombe force-pushed the build-yarn branch from 9c1a8d5 to deebebd Compare September 22, 2017 17:16

IgorMinar reviewed Sep 22, 2017

View reviewed changes

ocombe force-pushed the build-yarn branch from deebebd to 17baa78 Compare September 22, 2017 17:54

IgorMinar reviewed Sep 22, 2017

View reviewed changes

angular deleted a comment from mary-poppins Sep 22, 2017

ocombe force-pushed the build-yarn branch from 17baa78 to b47e407 Compare September 22, 2017 18:07

angular deleted a comment from mary-poppins Sep 22, 2017

ocombe force-pushed the build-yarn branch from b47e407 to a4163a0 Compare September 22, 2017 18:10

IgorMinar approved these changes Sep 22, 2017

View reviewed changes

ocombe added 2 commits September 22, 2017 20:47

build: switch from npm to yarn

2875a60

build: update npm dependencies

4976e0b

ocombe force-pushed the build-yarn branch from a4163a0 to 4976e0b Compare September 22, 2017 18:48

ocombe added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Sep 22, 2017

IgorMinar added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release and removed action: review The PR is still awaiting reviews from at least one requested reviewer PR target: TBD labels Sep 22, 2017

IgorMinar pushed a commit that referenced this pull request Sep 22, 2017

build: update npm dependencies (#19328)

0f5c70d

PR Close #19328

IgorMinar closed this in f48b343 Sep 22, 2017

gkalpak reviewed Sep 22, 2017

View reviewed changes

diagramatics mentioned this pull request Sep 25, 2017

build: use npm5 instead of yarn angular/angular-cli#7060

Merged

jmleoni pushed a commit to jmleoni/angular that referenced this pull request Oct 6, 2018

build: update npm dependencies (angular#19328)

117906e

PR Close angular#19328

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: switch from npm to yarn #19328

build: switch from npm to yarn #19328

ocombe commented Sep 22, 2017

mary-poppins commented Sep 22, 2017

mary-poppins commented Sep 22, 2017

4kochi commented Sep 22, 2017

ocombe commented Sep 22, 2017

4kochi commented Sep 22, 2017

mary-poppins commented Sep 22, 2017

IgorMinar Sep 22, 2017

ocombe Sep 22, 2017

IgorMinar Sep 22, 2017

ocombe Sep 22, 2017

IgorMinar Sep 22, 2017

IgorMinar Sep 22, 2017

IgorMinar Sep 22, 2017

mary-poppins commented Sep 22, 2017

alexeagle commented Sep 22, 2017

ocombe commented Sep 22, 2017

alexeagle commented Sep 22, 2017

gkalpak Sep 22, 2017

ocombe Sep 22, 2017

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

		@@ -37,7 +37,7 @@ fi
		setEnvVar NODE_VERSION 6.9.5
		setEnvVar NPM_VERSION 3.10.7 # do not upgrade to >3.10.8 unless https://github.com/npm/npm/issues/14042 is resolved

build: switch from npm to yarn #19328

build: switch from npm to yarn #19328

Conversation

ocombe commented Sep 22, 2017

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

mary-poppins commented Sep 22, 2017

mary-poppins commented Sep 22, 2017

4kochi commented Sep 22, 2017

ocombe commented Sep 22, 2017

4kochi commented Sep 22, 2017

mary-poppins commented Sep 22, 2017

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

mary-poppins commented Sep 22, 2017

alexeagle commented Sep 22, 2017

ocombe commented Sep 22, 2017

alexeagle commented Sep 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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