Skip to content
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

[Feature] Official support for Yarn #4317

Closed
3 tasks done
deepu105 opened this issue Oct 12, 2016 · 90 comments
Closed
3 tasks done

[Feature] Official support for Yarn #4317

deepu105 opened this issue Oct 12, 2016 · 90 comments

Comments

@deepu105
Copy link
Member

deepu105 commented Oct 12, 2016

The discussion started here

So people from FB, Google, Tilde etc have joinly released Yarn (read npm killer)

  • So for people who are frustrated with npm can just start using Yarn without needing to do any changes to your package.json in generated projects and generator itself. we dont have to do anything specific here (May be we can mention this in our docs)
  • This seems to work for a generated app (I tested only the angular2 branch, but should work on master as well) there are few minor quirks though
    1. Optional dependencies not met yarnpkg/yarn#628 (you need to run yarn add <lib-name> --dev for [imagemin-svgo, imagemin-jpegtran, imagemin-optipng, imagemin-gifsicle] as workaround)
    2. Package install fails on first try, success on second try consistently for many package yarnpkg/yarn#820 (just run add command again)
      So once this is stable we can may be check if yarn is present and then trigger yarn install instead of npm install post app generation, as its way way way faster. npm install can be fallback for people who ahve not installed yarn
  • Another step is to use yarn for our travis build as it would reduce run times greatly

@jhipster/developers what do you think, also it would be great if some of you can test this out on mac and linux

I would like to do some performance benchmark but im on a slow mobile network now so not sure if its ideal. So if someone on fast connection can benchmark that would be great

@sendilkumarn
Copy link
Member

@deepu105 as discussed in mail thread.
it took 80 sec to install everything for angular2 repo.

@sendilkumarn
Copy link
Member

no need of yarn install --> yarn would do

@deepu105
Copy link
Member Author

Yes I know, its just for consistency similar to we doing npm install where npm i would be sufficient

@deepu105
Copy link
Member Author

deepu105 commented Oct 12, 2016

@sendilkumarn could you do a benchmark?

  • create a fresh project
  • run npm install (measure actual time taken)
  • run npm install again (measure actual time taken)
  • delete the node_modules folder
  • run yarn (measure actual time taken not the one printed by yarn as its less than actual)
  • run yarn again (measure actual time taken not the one printed by yarn as its less than actual)

@PierreBesson
Copy link
Contributor

I'm all for supporting yarn when it is detected as globally installed. Yarm looks very promising and it's very simple to switch to it !
Let's try it first on Travis so that we can detect bugs if there are any...

@deepu105
Copy link
Member Author

@PierreBesson as soon as yarnpkg/yarn#789 is merged and released we can try it for our travis

@sendilkumarn
Copy link
Member

Npm install
first time - 3.19.65 ~ 200s
second time - 0.15.10 ~ 15s
Yarn install
first time - 1.10.69 ~ 71s
second time - 0.00.25 ~ 0.25s

@deepu105 the time is equal to what they show 😛

@cbornet
Copy link
Member

cbornet commented Oct 12, 2016

For a generator such as JHipster, this is a killer improvement !

@PierreBesson
Copy link
Contributor

On my machine (Macbook) measured using "time":

  • First npm install: 48.01s
  • Second npm install: 8.67s
  • rm node_modules
  • First yarn 28.78s
  • Second yarn 0.35s

@wmarques
Copy link
Contributor

On mine:

  • First npm i: 2m52.883s
  • Second npm i : 20.637s
  • removed node_modules
  • First yarn: 1m2.255s
  • second yarn: 0.29s.

@sendilkumarn
Copy link
Member

lets roll out yarn then.

@cbornet
Copy link
Member

cbornet commented Oct 12, 2016

For yarnpkg/yarn#789, maybe a workaround until the release:

In the meantime, adding the following to .yarnrc fixed the problem for me:
ignore-optional false

@deepu105
Copy link
Member Author

For a generated project this issue would be blocking yarnpkg/yarn#616
workaround is to run bower install after yarn install completes
For the generator and travis we can ofcourse try

@pascalgrimaud
Copy link
Member

pascalgrimaud commented Oct 13, 2016

I will test with our current Travis build

[edit] after some tries, I think I will wait the issue on yarn resolved 😪

@MathieuAA
Copy link
Member

Hey guys, they only had 1.3K stars 3 days ago, now they have 13K... that's crazy!

@deepu105
Copy link
Member Author

Big companies, lots of hype, more marketing and lots of frustrated npm
users :P

Thanks & regards,
Deepu

On 15 Oct 2016 01:04, "Mathieu ABOU-AICHI" notifications@github.com wrote:

Hey guys, they only had 1.3K stars 3 days ago, now they have 13K... that's
crazy!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4317 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABDlF_QuKzWhu7FXX31QBfaQ3n-xuw5iks5qz9lIgaJpZM4KUctC
.

@PierreBesson
Copy link
Contributor

We should be able to switch to yarn now that @sendilkumarn 's PR is merged and released upstream: https://github.com/yarnpkg/yarn/releases/tag/v0.16.0
Congrats Sendil !

@MathieuAA
Copy link
Member

Not yet... yarn's support is not widely "on" right now (check TravisCi PR about it here for instance). If you guys wanna use it, you can as it's on top of NPM but don't expect to see big differences... it's the opposite right now (check JHipster Core's yarn-dedicated branch, or better this). Too much hype kills the hype (but not the hipster :)).

@pascalgrimaud
Copy link
Member

Like @MathieuAA said, let's wait.
Yarn is not stable yet and it's not so easy to make it work with Travis.

Here what I tried:

  • 1st build without cache : all ok
  • 2nd build using the existing cache: fail with PhantomJS

I have one question: what is the equivalent of npm test ?

@MathieuAA
Copy link
Member

yarn run test... they haven't exposed everything yet.

@MathieuAA
Copy link
Member

You can see every command here.... but I've yet to see a project fully support the bare minimum as people do with NPM.

@pascalgrimaud
Copy link
Member

Thanks Matthieu! The last time I look, I didn't see it...

@deepu105
Copy link
Member Author

yes lets wait

@pascalgrimaud
Copy link
Member

I play with yarn since 0.16.1 and it seems working very well.
So for all projects generated, I use:

yo jhipster --skip-install
yarn && bower install && gulp install

What do you think about adding --yarn option?
The use will be: yo jhipster --yarn -> with this, the generator will do yarn install instead of npm install

I have this code already working on my branch, tell me if I should PR or not?
It should be a first step for supporting yarn in our generator without breaking the current behavior

@jdubois
Copy link
Member

jdubois commented Nov 3, 2016

Oh yes that would be cool!!!
We need this for our Devoxx presentation on Monday, with the bad Wifi!!
I would even add a new question for this.

Le 3 nov. 2016 7:35 PM, "Pascal Grimaud" notifications@github.com a
écrit :

I play with yarn since 0.16.1 and it seems working very well.
So for all projects generated, I use:

yo jhipster --skip-install
yarn && bower install && gulp install

What do you think about adding --yarn option?
The use will be: yo jhipster --yarn -> with this, the generator will do yarn
install instead of npm install

I have this code already working on my branch, tell me if I should PR or
not?
It should be a first step for supporting yarn in our generator without
breaking the current behavior


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#4317 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AATVow_4KiL4qB5R7N3GsMKNltlaKUqiks5q6ilfgaJpZM4KUctC
.

This was referenced Nov 3, 2016
@pascalgrimaud
Copy link
Member

pascalgrimaud commented Nov 18, 2016

How can we advance on it ?
I already have yarn working on Travis build and on maven-frontend-plugin. I didn't work on gradle yet.

I see 3 solutions:

  1. support both (npm and yarn) by adding a new question, and save yarn: false/true in .yo-rc.json file.
  2. migrate to yarn completely (less work, but risky as it is still in 0.X.X)
  3. wait :-)

@jdubois
Copy link
Member

jdubois commented Dec 18, 2016

If that's not a lot of work, here's my proposal:

  • We put Yarn by default, and tell everyone to use it
  • We watch the stats

-> if the stats show we have less than 5% of people using NPM, we can ditch it without any problem. That's what I did for Java 7, and that was very successful (= no complaint at all!)

@pascalgrimaud
Copy link
Member

  • should I add a new question for yarn / npm ?
  • or should I add a new flag --npm for people who want to stay with npm

@jdubois
Copy link
Member

jdubois commented Dec 18, 2016

Oh good question!

  • A question adds more visibility
  • A flag is only for advanced users

So if our final goal is to push Yarn, I would make it only a flag.

@gmarziou
Copy link
Contributor

gmarziou commented Dec 20, 2016

For developers, are we supposed to use yarn link rather than npm link?

@wmarques
Copy link
Contributor

@gmarziou yep

@PierreBesson
Copy link
Contributor

Yes and we should update the CONTRIBUTING.md file to document it.

@pascalgrimaud
Copy link
Member

@gmarziou :

  • in your local forked project (generator-jhipster): yarn link
  • when you want to generate a new project:
    • mkdir tmp && cd tmp
    • yarn link generator-jhipster
    • yo jhipster

@gmarziou
Copy link
Contributor

OK thanks

@jdubois
Copy link
Member

jdubois commented Dec 21, 2016

Can we close this? It's fully working for me!

@pascalgrimaud
Copy link
Member

pascalgrimaud commented Dec 21, 2016

There are few things to do like documentation, but we can follow this ticket #4703 for this
So yes!

@cbornet
Copy link
Member

cbornet commented Dec 21, 2016

Is it time to merge jhipster/jhipster-devbox#72 ?

@jdubois
Copy link
Member

jdubois commented Dec 21, 2016

@cbornet only once JHipster 4 is released, at the moment we need NPM

@jdubois
Copy link
Member

jdubois commented Dec 21, 2016

OK thanks @pascalgrimaud let's close this!

@jdubois jdubois closed this as completed Dec 21, 2016
@gmarziou
Copy link
Contributor

Shouldn't we add yarn.lock file to the generator repo?

@jdubois
Copy link
Member

jdubois commented Dec 22, 2016

For me it's the "good practice", so yes. But we haven't used this for long enough to be sure :-)

@cbornet
Copy link
Member

cbornet commented Dec 22, 2016

Are we sure we can do it for all combination of options ?

@jdubois
Copy link
Member

jdubois commented Dec 22, 2016

@cbornet I guess we can commit the one from the generator. Then, for each generated project, that's another story - yes we should commit it, but that's going to be a nightmare to generate... I would just let people commit it themselves after generation.

@deepu105
Copy link
Member Author

deepu105 commented Dec 22, 2016 via email

@allahbaksh
Copy link
Contributor

So what is the dev flow is mvn spring-boot:run and yarn?

@jdubois
Copy link
Member

jdubois commented Dec 26, 2016

@allahbaksh this is for JHipster 4, it's not released yet, and not documented yet

@allahbaksh
Copy link
Contributor

By the way if we install yarn through npm on MAC some time it has issues. Spent almost 3 hours on figuring out this.

@gmarziou
Copy link
Contributor

On Windows also there are issues, I could not make it work with nodist, I had to switch back to simpler standard installation of Node

@allahbaksh
Copy link
Contributor

allahbaksh commented Dec 27, 2016

webpack is throwing error var shouldExtract = !!(options.allChunks || chunk.isInitial());
TypeError: chunk.isInitial is not a function. Any workaround for this.

Got it worked by yarn add extract-text-webpack-plugin. Now have to resolve other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests