-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
|
@@ -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, [ | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly-stupid second question: There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
console.log([ | ||
__dirname, | ||
__filename | ||
]); |
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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
andbuiltins: false
ifnode: true
, and then just check fornode: true
here?There was a problem hiding this comment.
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, butopts.commondir
actually is the one causing the issue.