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

Upgrade to Yarn 1.0 #6362

Closed
jdubois opened this issue Sep 15, 2017 · 26 comments
Closed

Upgrade to Yarn 1.0 #6362

jdubois opened this issue Sep 15, 2017 · 26 comments
Milestone

Comments

@jdubois
Copy link
Member

jdubois commented Sep 15, 2017

Yarn 1.0 was just released, see https://github.com/yarnpkg/yarn/releases/tag/v1.0.0

Our current build works with the 0.27.5, but fails with 1.0.0. This needs to be fixed.

@jdubois
Copy link
Member Author

jdubois commented Sep 15, 2017

And maybe it's just me, but for developing JHipster, Yarn 1.0.0 is totally broken... yarn link generator-jhipster doesn't work correctly anymore:

  • After running it, I have errors in my build (saying there are some errors in generator-jhipster, like error An unexpected error occurred: "ENOENT: no such file or directory)
  • To solve this, I need to run yarn install in generator-jhipster
  • And after that, it is like yarn link didn't work, I have a non-development release of JHipster

Time to test NPM 5 again!!! Maybe move totally to NPM 5? It will ease installation, too.

@gmarziou
Copy link
Contributor

gmarziou commented Sep 15, 2017

npm5 works fine except that it's not packaged by default with node 6.x.

Also I found that on Fedora only yarn 1.0.0 was available while on Windows it was 1.0.1, so we should be careful to not require very latest version.

@gmarziou
Copy link
Contributor

gmarziou commented Sep 15, 2017

yarn link has some open issues with binaries (prior 1.0.0), so you might be better not using the cli but using yo.

@jdubois
Copy link
Member Author

jdubois commented Sep 15, 2017

Yes, but I'm using Node 8 without any trouble for some time now... I know we always recommend LTS, but maybe it's time to change. The main benefit for me is that there is one less thing to install for end-users - and for most Java developers, all this Node stuff is quite frightening.

Then, I would like to test what we can do with NPM 5. I have one big issue with our current build: it takes a long time to generate because it downloads some non-cacheable assets like Sass. Most of those are useless (unless you select Sass, of course), and come as transitive dependencies (Sass comes from Angular CLI), so I would like to be able to exclude them. I haven't found a way to do it yet, and this would speed up and ease the build for a lot of people. Also, I would love to be able to do an "offline build", that would be so helpful for demos and conferences where you have no Wifi.

@PierreBesson
Copy link
Contributor

PierreBesson commented Sep 15, 2017

Anyway, we still need to wait for a release of the front-end-maven plugin, so no hurry. Also personally, I have no problem building a JHipster app with my locally installed yarn 1.0.1.
[EDIT] Agree with the sass issue, it takes a long time and also breaks from time to time.

@gmarziou
Copy link
Contributor

gmarziou commented Sep 15, 2017

Node 8 will be LTS in October, so I'd prefer we wait for it so that our LTS version recommendation stays clear.

@pascalgrimaud
Copy link
Member

New release of frontend-maven-plugin: https://github.com/eirslett/frontend-maven-plugin/releases/tag/frontend-plugins-1.6
We should be able to upgrade this, @PierreBesson

@jdubois
Copy link
Member Author

jdubois commented Sep 25, 2017

I'm testing Yarn 1.1.0 and that looks better - I still get a few errors, but this is more usable -> I think we should migrate to it, as we can't stay with an old version anyway

@pascalgrimaud
Copy link
Member

I already started to work on this, but :

  • locally, it works well for me
  • it failed on Travis, I need time to investigate why

@jdubois
Copy link
Member Author

jdubois commented Sep 25, 2017

@pascalgrimaud I got the whole thing working quite well, but I didn't test on Travis.... I'll do a branch to test

@jdubois
Copy link
Member Author

jdubois commented Sep 25, 2017

@pascalgrimaud
Copy link
Member

There is an error during this phase: $ $JHIPSTER_SCRIPTS/02-generate-project.sh :

yarn install v0.27.5
info No lockfile found.
[1/4] Resolving packages...
warning browser-sync > browser-sync-ui > weinre > express@2.5.11: express 2.x series is deprecated
warning browser-sync > browser-sync-ui > weinre > express > connect@1.9.2: connect 1.x series is deprecated
[2/4] Fetching packages...
warning fsevents@1.1.2: The platform "linux" is incompatible with this module.
info "fsevents@1.1.2" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning "bootstrap@4.0.0-beta" has unmet peer dependency "popper.js@^1.11.0".
[4/4] Building fresh packages...
success Saved lockfile.
Done in 21.34s.
yarn run v0.27.5
$ yarn run cleanup && yarn run webpack:build:main
No configuration file found and no output filename configured via CLI option.
A configuration file could be named 'webpack.config.js' in the current directory.
Use --help to display the CLI options.
error Command failed with exit code 255.
error Command failed with exit code 1.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@jdubois
Copy link
Member Author

jdubois commented Sep 27, 2017

I'm still working on the https://github.com/jhipster/generator-jhipster/tree/yarn_1_1_0 branch
In fact the real issue is that Yarn does corrupt the cache, see yarnpkg/yarn#683 (we are not the only ones, and the last comments were a few hours ago, so that's definitely an issue).
I'm trying to disable concurrent Yarn builds, but then this is going to be really bad for performance, so I'm definitely wondering if migrating back to NPM would be a good idea.

@pascalgrimaud
Copy link
Member

Same for me. I'm wondering about switching back to NPM too... as I think you do the release with npm publish, right?
But switching back to NPM by default doesn't mean we won't support Yarn.
Our users will still be able to generate a project with Yarn, with jhipster --yarn (it works today with jhipster --npm)
But we need to discuss about this

@jdubois
Copy link
Member Author

jdubois commented Sep 27, 2017

Yes @pascalgrimaud I have 3 issues with Yarn:

  • This Travis bug, which is really huge, and which makes me wonder if Yarn has the level of quality we require
  • The dev environment which is broken: yarn link doesn't work half of the time (and that looks like the same bug, I have a corrupted cache), and this is very annoying
  • It is not the standard Node build tool: so yet another tool to install for our end-users, who generally don't care at all if we use NPM or Yarn (so that's more work for them, for nothing)

I had a quick look and migrating back to NPM seems to be quite a lot of work, unfortunately. And in that case, really, let's drop Yarn support: this is too much work to support both tools, for no added value (as our users don't care about that)

@pascalgrimaud
Copy link
Member

No worry, I coded the support for Yarn and switching back to NPM would be trivial.
I can do it if the team decide to switch back to NPM.

@PierreBesson
Copy link
Contributor

Sorry but I dont understand why we should remove yarn support just because of one bug ! Yes it's an annoying one but yarn just had their 1.0 release and this is the way of things in the JavaScript world, I predict you will have the same kind of breakage between released of npm. Also yarn is much faster in my experience and has helped tremendously with fixing unreliable transitive dependencies.

I'm also frustrated with yarn link issues for development but this should not be a reason to stop providing yarn support which works great for 100% of our users, just causing problem in Travis and for contributors.
Another argument for yarn is that it is not tied to node LTS versioning scheme.

@deepu105
Copy link
Member

Agree with @PierreBesson
Also, I'm having yarn 1.x and generated app works fine mostly so it's not affecting end users much. For dev we could, of course, use npm for link (Please note that you can mix both together as well, I for example always use npm link but for installation, I use yarn install)
We could also use npm on Travis until yarn issue is resolved
We could easily make npm the default option if required and use yarn if the flag is set similar to how npm is used today. I believe its a small change and I can do it if you want. Then we need to update docs which is mostly find and replace.

@deepu105
Copy link
Member

@jdubois I guess the changes to this file https://github.com/jhipster/generator-jhipster/compare/yarn_1_1_0?expand=1#diff-b80b01c33d5b4fd96f0304dbc1446060L137 is not required as passing -- works with both, with yarn it only prints a warning that its not required. Which IMO is fine if we make the change this will break for current Yarn version and will be a breaking change

@deepu105
Copy link
Member

Overall what I would suggest is

  1. Since NPM 5 Install is quite fast(not as fast as Yarn always) maybe we could make it the default again and have yarn as opt-in so that we don't have to worry much about such breakages. I believe the competition from Yarn will keep NPM on the edge always
  2. Keep current package.json code of app as it is so maximum versions are supported(In yarn 1.x it will just print a warning message which is ok IMO)
  3. Set NPM as default in Travis
  4. Update docs to instruct contributors to use NPM
  5. Update docs to use NPM commands instead of yarn as default

@jdubois
Copy link
Member Author

jdubois commented Sep 27, 2017

Thanks @deepu105 : indeed there's a warning with --, which I wanted to remove, but as you say this is going to break existing builds, so that's bad.
Then concerning this Yarn issue not affecting our end-users: I don't agree, as soon as they're going to run their app in Jenkins (using our great ci-cd subgen!), they'll have that issue.
I know @PierreBesson this is just a single bug, but this has blocked us for 2 weeks: when we have something this serious in JHipster, we usually fix it on the same day. And we are not backed by Facebook. For a build tool, we need something very stable and solid, and here I'm having doubts.

@jdubois
Copy link
Member Author

jdubois commented Sep 27, 2017

Sorry @deepu105 to annoy you again with that -- thing, but this is going to be a nightmare: if we remove it, people with older versions will have build failures. But if we don't, at some point there will be a new version that will break... I'm just getting tired of this :-(

@jdubois
Copy link
Member Author

jdubois commented Sep 27, 2017

And for the record my build broke again:

  • Using --mutex file fails
  • Using --mutex network fails
  • Using --network-concurrency 1 fails

So no solution for the moment

@pascalgrimaud
Copy link
Member

Hold on guys! I think I fixed the Travis build with Yarn 1.0 🤞
Let me test everything and we should not need to switch back to NPM by default

@jdubois
Copy link
Member Author

jdubois commented Sep 27, 2017

@deepu105
Copy link
Member

@jdubois about -- lets deal with it if it breaks, I mean future yarn will just pass it to underlying commands and If im not wrong most commands will just ignore it. Let me test that over the weekend

@jdubois jdubois added this to the 4.9.0 milestone Sep 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants