Skip to content

Commit

Permalink
fix(compartment-mapper): Dev implied by development condition only fo…
Browse files Browse the repository at this point in the history
…r entry package (#2641)

I discovered this bug while reviewing Compartment Mapper: I’ve
deprecated the `dev` option of `mapNodeModules` in favor of
`development` in the `conditions` set option, since generally one should
want both behaviors for development mode, so one should imply the other.
However, the way this is implemented causes `devDependencies` to be
linked for every package in the presence of the `development` condition,
and should only be in the entry package.
  • Loading branch information
kriskowal authored Nov 21, 2024
2 parents 7da1c44 + b7d7b23 commit 27ca207
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions packages/compartment-mapper/src/node-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ const graphPackage = async (
for (const name of Object.keys(optionalDependencies)) {
optionals.add(name);
}
if (dev !== undefined && dev !== null ? dev : conditions.has('development')) {
if (dev) {
assign(allDependencies, devDependencies);
}

Expand Down Expand Up @@ -849,7 +849,7 @@ const makeLanguageOptions = ({
/**
* @param {ReadFn | ReadPowers | MaybeReadPowers} readPowers
* @param {string} packageLocation
* @param {Set<string>} conditions
* @param {Set<string>} conditionsOption
* @param {object} packageDescriptor
* @param {string} moduleSpecifier
* @param {CompartmentMapForNodeModulesOptions} [options]
Expand All @@ -858,22 +858,29 @@ const makeLanguageOptions = ({
export const compartmentMapForNodeModules = async (
readPowers,
packageLocation,
conditions,
conditionsOption,
packageDescriptor,
moduleSpecifier,
options = {},
) => {
const { dev = undefined, commonDependencies = {}, policy } = options;
const { dev = false, commonDependencies = {}, policy } = options;
const { maybeRead, canonical } = unpackReadPowers(readPowers);
const languageOptions = makeLanguageOptions(options);

const conditions = new Set(conditionsOption || []);

// dev is only set for the entry package, and implied by the development
// condition.
// The dev option is deprecated in favor of using conditions, since that
// covers more intentional behaviors of the development mode.

const graph = await graphPackages(
maybeRead,
canonical,
packageLocation,
conditions,
packageDescriptor,
dev,
dev || (conditions && conditions.has('development')),
commonDependencies,
languageOptions,
);
Expand Down

0 comments on commit 27ca207

Please sign in to comment.