Opened on 08/23/2017 at 11:35:05 AM

Closed on 08/28/2017 at 08:55:36 AM

Last modified on 08/30/2017 at 02:24:33 PM

#5559 closed change (fixed)

Have install (production) Node.js dependencies when updating a dependency

Reported by: kzar Assignee: tlucas
Priority: P2 Milestone:
Module: Unknown Keywords:
Cc: trev, sebastian, tlucas Blocked By:
Blocking: #5535, #5560, #5596 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by trev)


We use JSDoc for document generation already, with #5535 we will use webpack for builds too. Unfortunately our dependency management system does not allow for Node.js npm dependencies to be specified and so the user has to manually install them globally instead.

We would like the script to call npm install --only=production --loglevel=warn after a dependency is updated which contains a package.json file. The --only=production argument avoids us installing development dependencies, for example adblockpluscore uses nodeunit for its tests which we don't want to install.

What to change

  • Change the script so that it calls npm install --only=production --loglevel=warn after a dependency is cloned or updated, but only if the following is true:
    • the dependency has a package.json file in its root
    • the package.json file has a non-empty "dependencies" section
    • the SKIP_DEPENDENCY_UPDATES environment variable isn't set.
  • The command should be called from the dependency's path.
  • If the npm command doesn't exist a warning should be displayed 'Failed to install Node.js dependencies for DEPENDENCY_NAME, please ensure Node.js is installed.'

Integration notes

When updating dependency on buildtools, this change and #5560 should be two separate dependency updates. This way will have support for Node.js dependencies once these dependencies are actually added.

Attachments (0)

Change History (18)

comment:1 Changed on 08/23/2017 at 11:42:57 AM by kzar

  • Description modified (diff)

comment:2 Changed on 08/23/2017 at 11:58:50 AM by kzar

  • Ready set

comment:3 Changed on 08/23/2017 at 01:11:47 PM by tlucas

What should happen in case npm is not available?

comment:4 Changed on 08/23/2017 at 01:31:31 PM by trev

A warning I guess, same as when hg isn't available for example.

comment:5 Changed on 08/23/2017 at 01:35:07 PM by kzar

  • Description modified (diff)

comment:6 Changed on 08/23/2017 at 01:36:53 PM by kzar

  • Description modified (diff)

comment:7 Changed on 08/23/2017 at 01:38:07 PM by trev

I'd suggest handling JSDoc in a separate change, it's not as trivial as adding a package.json file - it needs to be called differently as well.

comment:8 Changed on 08/23/2017 at 01:47:22 PM by kzar

  • Description modified (diff)

Fair enough, I removed the dependency. I'll open a separate issue for the JSDoc change now.

comment:9 Changed on 08/23/2017 at 01:56:52 PM by kzar

  • Blocking 5560 added

comment:10 Changed on 08/23/2017 at 02:07:19 PM by kzar

  • Description modified (diff)

comment:11 Changed on 08/23/2017 at 02:11:25 PM by kzar

  • Description modified (diff)

comment:12 Changed on 08/23/2017 at 06:04:59 PM by tlucas

  • Owner set to tlucas

comment:13 Changed on 08/24/2017 at 11:06:51 AM by tlucas

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:14 Changed on 08/25/2017 at 09:13:53 AM by tlucas

  • Description modified (diff)

comment:15 Changed on 08/25/2017 at 09:17:04 AM by trev

  • Description modified (diff)

comment:16 Changed on 08/28/2017 at 08:54:47 AM by abpbot

A commit referencing this issue has landed:
Issue 5559 - include Node.js in

comment:17 Changed on 08/28/2017 at 08:55:36 AM by tlucas

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:18 Changed on 08/30/2017 at 02:24:33 PM by kzar

  • Blocking 5596 added

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from tlucas.
Note: See TracTickets for help on using tickets.