Skip to content

Commit

Permalink
[FIX] npm t8r: Again, handle npm optionalDependencies correctly
Browse files Browse the repository at this point in the history
Fixes the fix done in #60.

Fixes: #51
  • Loading branch information
matz3 committed Nov 20, 2018
1 parent 7fe6269 commit 9fd78dc
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 22 deletions.
34 changes: 13 additions & 21 deletions lib/translators/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,6 @@ class NpmTranslator {
let dependencies = pkg.dependencies || {};
let optDependencies = pkg.devDependencies || {};

// Due to normalization done by by the "read-pkg-up" module the values
// in optionalDependencies get added to dependencies. Also as described here:
// https://github.com/npm/normalize-package-data#what-normalization-currently-entails
// Note: optionalDependencies are treated the same as devDependencies in the npm translator
if (pkg.optionalDependencies) {
for (const depName in pkg.optionalDependencies) {
if (pkg.optionalDependencies.hasOwnProperty(depName)) {
// Remove entry from "hard" dependencies map
if (dependencies[depName]) {
delete dependencies[depName];
}
// Add to optional dependencies map (of not already done)
if (optDependencies.hasOwnProperty(depName)) {
optDependencies[depName] = pkg.optionalDependencies[depName];
}
}
}
}

const version = pkg.version;

// Also look for "napa" dependencies (see https://github.com/shama/napa)
Expand Down Expand Up @@ -96,7 +77,8 @@ class NpmTranslator {
return this.getDepProjects({
cwd,
parentName: moduleName,
dependencies
dependencies,
optionalDependencies: pkg.optionalDependencies
}).then((depProjects) => {
// Array needs to be flattened because:
// getDepProjects returns array * 2 = array with two arrays
Expand Down Expand Up @@ -141,11 +123,21 @@ class NpmTranslator {
}
}

getDepProjects({cwd, dependencies, parentName}) {
getDepProjects({cwd, dependencies, optionalDependencies, parentName}) {
return Promise.all(
Object.keys(dependencies).map((moduleName) => {
return this.findModulePath(cwd, moduleName).then((modulePath) => {
return this.readProject({modulePath, moduleName, parentName});
}, (err) => {
// Due to normalization done by by the "read-pkg-up" module the values
// in optionalDependencies get added to dependencies. Also as described here:
// https://github.com/npm/normalize-package-data#what-normalization-currently-entails
// Resolution errors of optionalDependencies are being ignored
if (optionalDependencies && optionalDependencies[moduleName]) {
return null;
} else {
throw err;
}
});
})
).then((depProjects) => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions test/fixtures/application.g/node_modules/library.d/ui5.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/fixtures/application.g/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"description": "Simple SAPUI5 based application - test for npm optionalDependencies",
"main": "index.html",
"optionalDependencies": {
"library.d": "file:../library.d",
"library.nonexistent": "file:../library.nonexistent"
},
"scripts": {
Expand Down
9 changes: 8 additions & 1 deletion test/lib/translators/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,5 +238,12 @@ const applicationGTree = {
id: "application.g",
version: "1.0.0",
path: applicationGPath,
dependencies: []
dependencies: [
{
id: "library.d",
version: "1.0.0",
path: path.join(applicationGPath, "node_modules", "library.d"),
dependencies: []
}
]
};

0 comments on commit 9fd78dc

Please sign in to comment.