Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support monorepos #1677

Merged
merged 4 commits into from Jun 29, 2020
Merged

support monorepos #1677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d06ca51
support monorepos
micellius Apr 29, 2019
f2b118a
support monorepo (as fallback)
micellius Apr 29, 2019
66fc8fa
support monorepo (test case)
micellius Apr 29, 2019
5a0a02b
support monorepo (relative path)
micellius May 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion lib/grunt/task.js
View file
Open in desktop
Expand Up @@ -370,7 +370,23 @@ task.loadTasks = function(tasksdir) {
task.loadNpmTasks = function(name) {
loadTasksMessage('"' + name + '" local Npm module');
var root = path.resolve('node_modules');
var pkgfile = path.join(root, name, 'package.json');
var pkgpath = path.join(root, name);
var pkgfile = path.join(pkgpath, 'package.json');
// If package does not exist where grunt expects it to be,
// try to find it using Node's package path resolution mechanism
if (!grunt.file.exists(pkgpath)) {
var nameParts = name.split('/');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to use path.sep to support windows?

Looks like there are three more uses of / below too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redonculus , I tested my PR on windows and it works fine. There is a need to use path.sep only if someone is using syntax like require(".\\my\\foo\\index.js"). While this may work on windows, it fails on other platform. In opposite, using require("./my/foo/index.js") works on all platforms (including windows). So, practically, I don't see any reason someone would use windows specific paths, if there is a way to write cross-platform code with less typing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok that’s fine then. If it works on windows then I’m good.

// In case name points to directory inside module,
// get real name of the module with respect to scope (if any)
var normailzedName = (name[0] === '@' ? nameParts.slice(0,2).join('/') : nameParts[0]);
try {
pkgfile = require.resolve(normailzedName + '/package.json');
root = pkgfile.substr(0, pkgfile.length - normailzedName.length - '/package.json'.length);
} catch (err) {
grunt.log.error('Local Npm module "' + normailzedName + '" not found. Is it installed?');
return;
}
}
var pkg = grunt.file.exists(pkgfile) ? grunt.file.readJSON(pkgfile) : {keywords: []};

// Process collection plugins.
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/load-npm-tasks/test-package/package.json
View file
Open in desktop
@@ -0,0 +1,7 @@
{
"private": true,
"name": "test-package",
"devDependencies": {
"grunt-foo-plugin": "1.0.0"
}
}
11 changes: 9 additions & 2 deletions test/gruntfile/load-npm-tasks.js
View file
Open in desktop
Expand Up @@ -4,8 +4,8 @@ var Log = require('grunt-legacy-log').Log;
var assert = require('assert');
var through = require('through2');

module.exports = function(grunt) {
grunt.file.setBase('../fixtures/load-npm-tasks');
function test(grunt, fixture) {
grunt.file.setBase('../fixtures/' + fixture);

// Create a custom log to assert output
var stdout = [];
Expand Down Expand Up @@ -39,4 +39,11 @@ module.exports = function(grunt) {
throw err;
}
});
}

module.exports = function(grunt) {
// NPM task package is inside $CWD/node_modules
test(grunt, 'load-npm-tasks');
// NPM task package hoisted to $CWD/../node_modules
test(grunt, 'load-npm-tasks/test-package');
};