Skip to content

Commit

Permalink
Merge branch 'improve-require-file-name'
Browse files Browse the repository at this point in the history
Fix fileName for functions defined by modules loaded using require().
Although the fileName for the (Duktape internal) module function was set
to the resolved module ID, any functions defined in the module woult not
be affected by this and fileName would default to "duk_bi_global.c" which
is highly misleading.  See GH-58.
  • Loading branch information
svaarala committed Oct 29, 2014
2 parents b0a553a + 3a63e10 commit 624831f
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 9 deletions.
4 changes: 4 additions & 0 deletions RELEASES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,10 @@ Planned

* Fix compile error for DUK_OPT_NO_PC2LINE

* Fix fileName for functions defined in a module loaded using require(),
previously fileName would always be duk_bi_global.c which is misleading
(see GH-58)

1.2.0 (2015-XX-XX)
------------------

Expand Down
6 changes: 6 additions & 0 deletions ecmascript-testcases/test-bug-date-timeval-edges.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
* to either side of 01 January, 1970 UTC.
*/

/*---
{
"knownissue": "test case depends on current timezone offset"
}
---*/

/*===
test1
8639999996399999
Expand Down
50 changes: 50 additions & 0 deletions ecmascript-testcases/test-commonjs-require-filename.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Filename for functions inside a module loaded using require()
*
* In Duktape 1.0.0 this would always be "duk_bi_global.c" which is confusing.
* For Duktape 1.1.0 this was fixed to be the fully resolved module ID.
* See GH-58 for discussion.
*/

/*---
{
"custom": true
}
---*/

/*===
moduleFunc name:
moduleFunc fileName: foo
testFunc name: testFunc
testFunc fileName: foo
===*/

function modSearch() {
return "function testFunc() { print('testFunc name:', Duktape.act(-2).function.name);\n" +
" print('testFunc fileName:', Duktape.act(-2).function.fileName); }\n" +
"\n" +
"exports.testFunc = testFunc;\n" +
"print('moduleFunc name:', Duktape.act(-2).function.name);\n" +
"print('moduleFunc fileName:', Duktape.act(-2).function.fileName);\n";
}

function test() {
/* For the module function itself (which is a Duktape internal artifact)
* the fileName is already forced to be module ID. This was OK in
* Duktape 1.0.0.
*/

Duktape.modSearch = modSearch;
var mod = require('foo');

/* However, functions defined within the module don't have a proper
* fileName in Duktape 1.0.0.
*/
mod.testFunc();
}

try {
test();
} catch (e) {
print(e.stack || e);
}
16 changes: 7 additions & 9 deletions src/duk_bi_global.c
Original file line number Diff line number Diff line change
Expand Up @@ -1073,17 +1073,15 @@ DUK_INTERNAL duk_ret_t duk_bi_global_object_require(duk_context *ctx) {
return 1;
}

/* Finish the wrapped module source. */
/* Finish the wrapped module source. Force resolved module ID as the
* fileName so it gets set for functions defined within a module. This
* also ensures loggers created within the module get the module ID as
* their default logger name.
*/
duk_push_string(ctx, "})");
duk_concat(ctx, 3);
duk_eval(ctx);

/* Force 'fileName' property of the module function so that if the
* module creates a logger, the logger name defaults to the module
* name.
*/
duk_dup(ctx, 3);
duk_put_prop_stridx(ctx, -2, DUK_STRIDX_FILE_NAME);
duk_dup(ctx, 3); /* resolved module ID for fileName */
duk_eval_raw(ctx, NULL, 0, DUK_COMPILE_EVAL);

/* XXX: The module wrapper function is currently anonymous and is shown
* in stack traces. It would be nice to force it to match the module
Expand Down

0 comments on commit 624831f

Please sign in to comment.