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.

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

Dynamically calculate __dirname and __filename when --node is passed #1725

Merged
merged 1 commit into from Feb 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 16 additions & 2 deletions index.js
@@ -1,3 +1,4 @@
var path = require('path');
var mdeps = require('module-deps');
var depsSort = require('deps-sort');
var bpack = require('browser-pack');
Expand Down Expand Up @@ -551,6 +552,19 @@ Browserify.prototype._createDeps = function (opts) {
return through();
}
}

if (opts.commondir === false && opts.builtins === false) {
opts.insertGlobalVars = xtend({
Copy link
Member

Choose a reason for hiding this comment

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

could this be done in cmd/args.js, in the same place as the other node: option stuff?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure I understand this comment. Do you mean setting commondir: false and builtins: false if node: true, and then just check for node: true here?

Copy link
Member

Choose a reason for hiding this comment

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

oh sorry nevermind, i misunderstood what this check is doing--i thought you were using this to essentially check if --node was passed, but opts.commondir actually is the one causing the issue.

__dirname: function(file, basedir) {
var dir = path.dirname(path.relative(basedir, file));
return 'require("path").join(__dirname,' + dir.split(path.sep).map(JSON.stringify).join(',') + ')';
},
__filename: function(file, basedir) {
var filename = path.relative(basedir, file);
return 'require("path").join(__dirname,' + filename.split(path.sep).map(JSON.stringify).join(',') + ')';
}
}, opts.insertGlobalVars);
}

var vars = xtend({
process: function () { return "require('_process')" },
Expand All @@ -560,11 +574,11 @@ Browserify.prototype._createDeps = function (opts) {
vars.process = undefined;
vars.buffer = undefined;
}

return insertGlobals(file, xtend(opts, {
debug: opts.debug,
always: opts.insertGlobals,
basedir: opts.commondir === false
basedir: opts.commondir === false && isArray(opts.builtins)
? '/'
: opts.basedir || process.cwd()
,
Expand Down
78 changes: 77 additions & 1 deletion test/bare.js
Expand Up @@ -3,6 +3,10 @@ var spawn = require('child_process').spawn;
var path = require('path');
var concat = require('concat-stream');
var vm = require('vm');
var fs = require('fs');
var temp = require('temp');
temp.track();
var tmpdir = temp.mkdirSync({prefix: 'browserify-test'});

test('bare', function (t) {
t.plan(4);
Expand Down Expand Up @@ -41,14 +45,17 @@ test('bare', function (t) {
test('bare inserts __filename,__dirname but not process,global,Buffer', function (t) {
t.plan(2);

var file = path.resolve(__dirname, 'bare/main.js');
var ps = spawn(process.execPath, [
path.resolve(__dirname, '../bin/cmd.js'),
path.resolve(__dirname, 'bare/main.js'),
file,
'--bare'
]);

ps.stdout.pipe(concat(function (body) {
vm.runInNewContext(body, {
require: require,
__dirname: process.cwd(),
console: {
log: function (msg) {
t.same(msg, [
Expand All @@ -68,3 +75,72 @@ test('bare inserts __filename,__dirname but not process,global,Buffer', function
t.equal(code, 0);
});
});

test('bare inserts dynamic __filename,__dirname', function (t) {
Copy link

Choose a reason for hiding this comment

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

Possibly-stupid second question:
The test description mentions both __filename and __dirname, but AFAICT this is only testing __dirname?

Copy link
Author

Choose a reason for hiding this comment

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

These tests execute the following file: https://github.com/substack/node-browserify/pull/1725/files#diff-df4230a729c19a353d1405d5459df60d, which prints both __dirname, and __filename (and we assert the result of both)

t.plan(2);

var file = path.join(tmpdir, 'dirname-filename.js');

fs.writeFileSync(
file,
fs.readFileSync(path.resolve(__dirname, 'bare/dirname-filename.js'))
);

var ps = spawn(process.execPath, [
path.resolve(__dirname, '../bin/cmd.js'),
file,
'--bare'
]);

ps.stdout.pipe(concat(function (body) {
vm.runInNewContext(body, {
require: require,
__dirname: path.dirname(file),
console: {
log: function (msg) {
t.same(msg, [
path.dirname(file),
file
]);
}
}
});
}));
ps.stdin.end();

ps.on('exit', function (code) {
t.equal(code, 0);
});
});

test('bare inserts dynamic __filename,__dirname with basedir', function (t) {
t.plan(2);

var file = 'dirname-filename.js';
var ps = spawn(process.execPath, [
path.resolve(__dirname, '../bin/cmd.js'),
file,
'--bare',
'--basedir=' + path.join(__dirname, 'bare')
]);

ps.stdout.pipe(concat(function (body) {
vm.runInNewContext(body, {
require: require,
__dirname: process.cwd(),
console: {
log: function (msg) {
t.same(msg, [
__dirname,
path.join(__dirname, file)
]);
}
}
});
}));
ps.stdin.end();

ps.on('exit', function (code) {
t.equal(code, 0);
});
});
4 changes: 4 additions & 0 deletions test/bare/dirname-filename.js
@@ -0,0 +1,4 @@
console.log([
__dirname,
__filename
]);
9 changes: 6 additions & 3 deletions test/no_builtins.js
@@ -1,12 +1,14 @@
var browserify = require('../');
var test = require('tap').test;
var path = require('path');
var vm = require('vm');

test('builtins false', function (t) {
t.plan(1);


var file = __dirname + '/no_builtins/main.js';
var b = browserify({
entries: [ __dirname + '/no_builtins/main.js' ],
entries: [ file ],
commondir: false,
builtins: false
});
Expand All @@ -15,7 +17,8 @@ test('builtins false', function (t) {
console: { log: function (msg) {
t.equal(msg, 'beep boop\n');
} },
require: require
require: require,
__dirname: process.cwd()
};
vm.runInNewContext(src, c);
});
Expand Down