[keycloak-dev] Merging node_modules into Keycloak repo

Thu May 11 21:25:26 EDT 2017

Using yarn instead of npm to manage node_modules (it's a drop-in
replacement in most cases) will provide a local package cache similar to
the local Maven repo: https://yarnpkg.com

-Steven

On Thu, May 11, 2017, 5:57 PM Stan Silvert <ssilvert at redhat.com> wrote:

> On 5/11/2017 4:06 PM, Bruno Oliveira wrote:
> > Some answers inline.
> More comments inline...
> >
> > On 2017-05-11, Stian Thorgersen wrote:
> >> Oh and also I like to just run stuff from the IDE. I very rarely build
> >> Keycloak from mvn unless I'm mocking about with the distribution parts.
> > To run stuff from your IDE, you just run npm install inside the project.
> > You don't need Maven for it.
> >
> >> On 11 May 2017 at 19:17, Stian Thorgersen <sthorger at redhat.com> wrote:
> >>
> >>> I have 3 concerns around not checking in node_modules. If these are
> >>> addressed I'm happy:
> >>>
> >>> * Productization - I believe not checking in the modules will mean they
> >>> have to be available in some internal NPM repository or something.
> Those
> >>> module not already available we'll have to add ourselves. Stan you
> would
> >>> have to figure this out as we no longer have a dedicated prod team
> (that's
> >>> us now!)
> > Yep, I know. One of the reasons why run diff at each upgrade of these
> > dependencies will sound like hell.
> I don't know the answer to the productization question.  SOMEBODY in
> MiddleWare
> must have dealt with this before.  Who can I ask?
> >
> >>> * Build time - just the basic Angular2 example adds overhead
> > I won't take the road of Java vs Node.js. But pretty much you have the
> > same issue with Maven, when you don't have dependencies installed.
> > One way or another you have to download it from the internet.
> With Maven, you only have to download once.  But with node_modules not
> checked in
> you will have to download every time after you do a clean.  Or is there
> some way around that?
>
> npm doesn't seem to have the same concept of a local repo like Maven.
> Installing globally doesn't fix
> the problem.  Or is there a solution I don't know about?
> >
> >>> * Offline mode - the Angular2 example at the moment can't be built when
> >>> not online. It will sit around for a loooong time until it eventually
> times
> >>> out. NPM modules would have to be checked in somewhere globally (not
> just
> >>> inside target dir or something like that). Maybe they can just be
> placed
> >>> inside the .m2 dir or something?!
> > Isn't the same when you are disconnected and you try to run Maven?
> No.   You can rely on the local Maven repo.  Works great disconnected.
> >
> > Anyways, I don't think we gonna reach any consensus about it. So,
> anything
> > you guys think is the best for the project, go ahead. It's software, we
> > can always change, sometimes break :)
> >
> >>> On 11 May 2017 at 15:10, Bruno Oliveira <bruno at abstractj.org> wrote:
> >>>
> >>>> On 2017-05-11, Stan Silvert wrote:
> >>>>> On 5/10/2017 6:14 PM, Bruno Oliveira wrote:
> >>>>>> On 2017-05-10, Stan Silvert wrote:
> >>>>>>> The very thought of $subject seems like heresy.  Why check in
> >>>> something
> >>>>>>> that is normally pulled using npm?
> >>>>>>>
> >>>>>>> We have Angular 2 examples in Keycloak now.  In the not-too-distant
> >>>>>>> future, our Account Management console will be written in Angular
> >>>> 2.  So
> >>>>>>> node_modules has to be there somehow.
> >>>>>>>
> >>>>>>> There are basically two options:
> >>>>>>> 1)  Merge node_modules into the Keycloak repo.
> >>>>>>> 2)  Don't merge and then run npm install at build time.
> >>>>>>>
> >>>>>>> Productization standards push toward option #1.  We need to have
> >>>>>>> consistent, repeatable builds.
> >>>>>>>
> >>>>>>> But I'm looking for reasons that #1 might be bad.  I can't come up
> >>>> with
> >>>>>>> a rational reason to do #2 except that it saves disk space.
> >>>>>> My 2 cents here. I think #1 is bad for the following reasons:
> >>>>> My instinct is that it's bad too.  But I need to play devil's
> advocate.
> >>>>>> 1. That's not the convention for Node.js development. Think about
> Java
> >>>>>> devs committing JARs to our repo. Certainly that would be terrible.
> >>>>> That's just the heresy argument.
> >>>> Not really. It's also the way which QE thinks is the way to go (
> >>>> https://github.com/keycloak/keycloak-quickstarts/pull/25#is
> >>>> suecomment-300751341)
> >>>>
> >>>>>> 2. Code review becomes a PITA at every dependency update.
> >>>>> If you are talking about review in GitHub, that shouldn't be a
> problem.
> >>>>> GitHub uses linguist to hide vendor files so that it doesn't mess up
> >>>> your
> >>>>> review.  All you see is the file name.  By default, it doesn't show
> you
> >>>> the
> >>>>> whole file.  Strangely enough, the linguist guys see checking in js
> >>>>> libraries as a common practice:
> >>>>> https://github.com/github/linguist#vendored-code
> >>>> Well, you're assuming that we should all do our code reviews using
> only
> >>>> GH.
> >>>>
> >>>>>> 3. Merge conflicts. Just think about multiple devs contribution to
> the
> >>>>>> same repo and updating their modules.
> >>>>> Maybe I'm missing something here.  Wouldn't that be really rare if it
> >>>> ever
> >>>>> happened at all (in our case)?
> >>>> Not if every developer decide to run npm update :)
> >>>>
> >>>>>> 4. People could manually change these modules and you would never
> know
> >>>>>> if the change is a consequence of `npm update` or a manual change.
> >>>>> I really hope nobody would be dumb enough to do that!
> >>>>>
> >>>>> But come to think of it, this would be a security issue if someone
> did
> >>>> it on
> >>>>> purpose.   Someone could sneak malicious code into our repo disguised
> >>>> as a
> >>>>> legit PR.  It wouldn't be easy to catch during code review.
> >>>> Trust me, if people wanted to do that. They wouldn't be dumb just
> >>>> a single line
> >>>>
> >>>>>> 5. This only makes sense on scenarios where dev cannot rely on npm
> >>>>>> dependencies.
> >>>>>>
> >>>>>>
> >>>>>> IMO option #2 would be the most viable. Projects from RedHat already
> >>>> do
> >>>>>> this[1] with some success. So I don't see any need for it.
> >>>>>>
> >>>>>> [1] - https://github.com/aerogear/aerogear-unifiedpush-server/blob
> >>>> /02b133ffb49677effa347788c28c392ed3f275f0/admin-ui/pom.xml#L42
> >>>>> Doesn't this make the build a LOT slower?
> >>>> If slowness is the major concern, we should commit every single JAR
> >>>> inside the repo and call it a day. But I don't think we do that,
> right?
> >>>> Plus, Travis can cache the dependencies[1].
> >>>>
> >>>> [1] - https://docs.travis-ci.com/user/caching/
> >>>>
> >>>>> How is Aerogear dealing with productization?
> >>>> You have to ask them, more precisely Matthias.
> >>>>
> >>>>>>
> >>>>>>> Any thoughts?
> >>>>>>> _______________________________________________
> >>>>>>> keycloak-dev mailing list
> >>>>>>> keycloak-dev at lists.jboss.org
> >>>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >>>>>> --
> >>>>>>
> >>>>>> abstractj
> >>>> --
> >>>>
> >>>> abstractj
> >>>> _______________________________________________
> >>>> keycloak-dev mailing list
> >>>> keycloak-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >>>>
> >>>
> > --
> >
> > abstractj
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>