Make WordPress Core

Opened 4 years ago

Closed 7 months ago

#51784 closed enhancement (maybelater)

Build/Test Tools: Consider always running npm install with --no-optional

Reported by: azaozz's profile azaozz Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: 2nd-opinion has-patch close
Focuses: Cc:

Description

Currently there are 248 optional third (and forth, fifth) party dependencies in package-lock.json that don't seem needed.

Running npm install --no-optional seems to still install many of them (presumably as they are needed by other dependencies) but reduces the overall size of node_modules by about 40MB and speeds things up a bit.

Attachments (1)

51784.diff (297 bytes) - added by azaozz 4 years ago.

Download all attachments as: .zip

Change History (5)

#1 @azaozz
4 years ago

Similarly dev-dependencies of third party packages (and sub-dependency packages) don't seem needed either but there isn't a clear way to skip these for the moment (as far as I see). Doing --only=prod would skip top-level (our own) dev dependencies too.

@azaozz
4 years ago

#2 @azaozz
4 years ago

  • Keywords 2nd-opinion has-patch added

In 51784.diff: Set npm to skip optional dependencies.

However that may not be good, see the corresponding issue for Gutenberg: https://github.com/WordPress/gutenberg/issues/26993.

#3 @desrosj
14 months ago

  • Keywords close added

Just noting that --omit=optional and --include=optional are now the preferred flags for achieving what we're looking for here. The old form ones listed above now produce warnings.

I'm going to mark this as a close candidate with a maybelater resolution.

I am personally against adding a blanket option for skipping optional dependencies because it's such a rabbit hole and there are likely going to be unknown side effects that change every time package versions are bumped. These edge cases will also end up being different for different version branches over time, which sounds like a real headache. I prefer to defer to npm for determining which packages are appropriate to install as much as possible.

However, I would be in favor of adding some details to the README file that explain the benefits of the --omit=optional option for any contributors that would like to utilize it. If they want, they can define this in their per-user config file, or simply just include --omit=optional when running the npm install command.

In general, I think we should be more mindful of the packages we're utilizing, how they're being used, and what the broader impacts are for contributors.

Leaving this open for feedback from other contributors with an interest, but if there's no strong disagreement in 2-3 months, I'll circle back to close this out.

#4 @jorbin
7 months ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

I agree with @desrosj and based on the lack of disagreement, I am going to close this out. The edge cases and rabbit holes this could present aren't worth the potential benefit.

Note: See TracTickets for help on using tickets.