The `p < nmLen` condition will fail when a module's name is end with
`node_modules` like `foo_node_modules`. The old logic will miss the
`foo_node_modules/node_modules` in node_modules paths.
TL;TR, a module named like `foo_node_modules` can't require any module
in the node_modules folder.
Fixes: https://github.com/nodejs/node/issues/6679
PR-URL: https://github.com/nodejs/node/pull/6670
Reviewed-By: Evan Lucas <evanlucas@me.com>
This change is in preparation for lint-enforced brace style.
PR-URL: https://github.com/nodejs/node/pull/7630
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Add the `--preserve-symlinks` flag. This makes the changes added
in #5950 conditional. By default the old behavior is used. With
the flag set, symlinks are preserved, switching to the new
behavior. This should be considered to be a temporary solution
until we figure out how to solve the symlinked peer dependency
problem in a more general way that does not break everything
else.
Additional test cases are included.
PR-URL: https://github.com/nodejs/node/pull/6537
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Minor cleanup of how --debug-brk works:
* We no longer need to use command line flags to expose the debug
object.
* Do not depend on the existence of global.v8debug as a mechanism to
determine if --debug-brk was specified.
* We no longer need to set a dummy listener with --debug-brk.
PR-URL: https://github.com/nodejs/node/pull/6599
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Currently, required modules use the real location of the
package/file as their __filename and __dirname, instead
of the symlinked path if it exists. This behaviour is
undocumented (it even goes against documentation in
certain scenarios), creating hard-to-debug problems
for developers who wish to leverage filesystem abstractions
to lay out their application.
This patch resolves all required modules to their canonical
path while still preserving any symlinks within the path,
instead of resolving to their canonical realpath. The one
special case observed is when the main module is loaded
-- in this case, the realpath does need to be used
in order for the main module to load properly.
PR-URL: https://github.com/nodejs/node/pull/5950
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove realpath() and realpathSync() cache.
Use the native uv_fs_realpath() which is faster
then the JS implementation by a few orders of magnitude.
PR-URL: https://github.com/nodejs/node/pull/3594
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
This commit improves module loading performance by at least ~25-35%
in the module-loader benchmarks.
Some optimization strategies include:
* Try-finally/try-catch isolation
* Replacing regular expressions with manual parsing
* Avoiding unnecessary string and array creation
* Avoiding constant recompilation of anonymous functions and
function definitions within functions
PR-URL: https://github.com/nodejs/node/pull/5172
Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes a bug where a 3rd party module found in node_modules,
would be preferred over a ./local module with the same name.
Fixes: https://github.com/nodejs/node/issues/5684
PR-URL: https://github.com/nodejs/node/pull/5689
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
`homedir` was declared with `var` twice in the same scope in
`lib/module.js`. This change makes it a single declaration.
PR-URL: https://github.com/nodejs/node/pull/4962
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
The vm module's displayErrors option attaches error arrow
messages as a hidden property. Later, core JavaScript code
can optionally decorate the error stack with the arrow message.
However, when user code catches an error, it has no way to
access the arrow message. This commit changes the behavior of
displayErrors to mean "decorate the error stack if an error
occurs."
Fixes: https://github.com/nodejs/node/issues/4835
PR-URL: https://github.com/nodejs/node/pull/4874
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Use internalModuleReadFile() to read the file from disk to avoid the
fs.fstatSync() call that fs.readFileSync() makes.
It does so to know the file size in advance so it doesn't have to
allocate O(n) buffers when reading the file from disk.
internalModuleReadFile() is plenty efficient though, even more so
because we want a string and not a buffer. This way we also don't
allocate a buffer that immediately gets thrown away again.
This commit reduces the number of fstat() system calls in a benchmark
application[0] from 549 to 29, all made by the application itself.
[0] https://github.com/strongloop/loopback-sample-app
PR-URL: https://github.com/nodejs/node/pull/4575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Avoid an unneeded ArgumentsAdaptorTrampoline stack frame by passing the
the right number of arguments to Module._load() in Module.require().
Shortens the following stack trace with one frame:
LazyCompile:~Module.load module.js:345
LazyCompile:Module._load module.js:282
Builtin:ArgumentsAdaptorTrampoline
LazyCompile:*Module.require module.js:361
LazyCompile:*require internal/module.js:11
PR-URL: https://github.com/nodejs/node/pull/4575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reduce the number of stat() system calls that require() makes by caching
the results more aggressively.
To avoid unbounded growth without implementing a LRU cache, scope the
cache to the lifetime of the first call to require(). Recursive calls
(i.e. require() calls in the included code) transparently profit from
the cache.
The benchmarked application is the loopback-sample-app[0] and it sees
the number of stat calls at start-up go down by 40%, from 4736 to 2810.
[0] https://github.com/strongloop/loopback-sample-app
PR-URL: https://github.com/nodejs/node/pull/4575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The exts and trailingSlash variables are only used if the
path isn't cached. This commit moves them further down in the
code, and changes from var to const.
PR-URL: https://github.com/nodejs/node/pull/3579
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Remove a hack that was introduced in commit bb6d468d from November 2010.
This is groundwork for a follow-up commit that makes it possible to use
internal modules in lib/repl.js.
PR-URL: https://github.com/nodejs/node/pull/4026
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Because Node modules are wrapped, errors on the first line
of a file leak the wrapper to the user and report the wrong
column number. This commit adds a line break to the module
wrapper so that the first line is treated the same as all
other lines. To compensate for the additional line, a line
offset of -1 is also applied to errors.
Fixes: https://github.com/nodejs/node/issues/2860
PR-URL: https://github.com/nodejs/node/pull/2867
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
None of these regular expressions will change, so there is no need to
generate them every time in hot code paths.
Provides a small performance improvement in module loading. (5-10%)
PR-URL: https://github.com/nodejs/node/pull/3869
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`debuglog` uses `%j` as a placeholder for replacement with
`JSON.stringify`. So that `JSON.stringify` is only called when the
appropriate debug flag is on. The other `%s` changes are for style
consistency.
PR-URL: https://github.com/nodejs/node/pull/3578
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`require.paths` property and `require.registerExtension` function have
been throwing errors when used. They both are like this for years now.
This patch removes them from the system.
PR-URL: https://github.com/nodejs/node/pull/2922
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
When using require to load a native addon the path must be converted
into a long path, otherwise the addon will fail to be loaded on
windows if the path is longer than 260 characters.
PR-URL: https://github.com/nodejs/node/pull/2965
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <inglor@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/2411
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Changes included in this commit are
1. Making the deprecation messages consistent. The messages will be in
the following format
x is deprecated. Use y instead.
If there is no alternative for `x`, then the ` Use y instead.` part
will not be there in the message.
2. All the internal deprecation messages are printed with the prefix
`(node) `, except when the `--trace-deprecation` flag is set.
Fixes: https://github.com/nodejs/io.js/issues/1883
PR-URL: https://github.com/nodejs/io.js/pull/1892
Reviewed-By: Roman Reiss <me@silverwind.io>
require() now checks that the path exists before searching
further in it.
PR-URL: https://github.com/nodejs/io.js/pull/1920
Reviewed-By: Isaac Z. Schlueter <i@izs.me>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
When the preload module is not a abs/relative path, we should use
the standard search mechanism of looking into the node_modules folders
outwards. The current working directory is deemed to be the 'requiring
module', i.e. parent. The search path starts from cwd outwards.
Fixes: https://github.com/nodejs/io.js/issues/1803
PR-URL: https://github.com/nodejs/io.js/pull/1812
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Replace calls to fs.readFileSync() with an internal variant that does
not create Error objects on failure and is a bit speedier in general.
A secondary benefit is that it improves start-up times in the debugger
because it no longer emits thousands of exception debug events.
On a medium-sized application[0], this commit and its predecessor reduce
start-up times from about 1.5s to 0.5s and reduce the number of start-up
exceptions from ~6100 to 32, half of them internal to the application.
[0] https://github.com/strongloop/loopback-sample-app
PR-URL: https://github.com/nodejs/io.js/pull/1801
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Replace calls to fs.statSync() with an internal variant that does not
create Error or Stat objects that put strain on the garbage collector.
A secondary benefit is that it improves start-up times in the debugger
because it no longer emits thousands of exception debug events.
PR-URL: https://github.com/nodejs/io.js/pull/1801
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
this creates a new internal module responsible for providing
the repl created via "iojs" or "iojs -i," and adds the following
options to the readline and repl subsystems:
* "repl mode" - determine whether a repl is strict mode, sloppy mode,
or auto-detect mode.
* historySize - determine the maximum number of lines a repl will store
as history.
The built-in repl gains persistent history support when the
NODE_REPL_HISTORY_FILE environment variable is set. This functionality
is not exposed to userland repl instances.
PR-URL: https://github.com/iojs/io.js/pull/1513
Reviewed-By: Fedor Indutny <fedor@indutny.com>
If `$NODE_PATH` contains trailing separators, `Module.globalPaths` will
contains empty strings. When `Module` try to resolve a module's path,
`path.resolve('', 'index.js')` will boil down to `$PWD/index.js`, which
makes sub modules can access global modules and get unexpected result.
PR-URL: https://github.com/iojs/io.js/pull/1488
Reviewed-By: Roman Reiss <me@silverwind.io>
This commit restores the functionality of adding a module's path to
NODE_PATH and requiring it with require('.'). As NODE_PATH was never
intended to be used as a pointer to a module directory (but instead, to
a directory containing directories of modules), this feature is also
being deprecated in turn, to be removed at a later point in time.
PR-URL: https://github.com/iojs/io.js/pull/1363
Fixes: https://github.com/iojs/io.js/issues/1356
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Internal modules can be used to share private code between
public modules without risk to expose private APIs to the
user.
PR-URL: https://github.com/iojs/io.js/pull/848
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Previously, the minimal argument to require the current directory was
require('./'). This commits allows to skip the trailing slash.
Fixes: https://github.com/iojs/io.js/issues/1178
PR-URL: https://github.com/iojs/io.js/pull/1185
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Christian Tellnes <christian@tellnes.no>
Reviewed-By: Roman Reiss <me@silverwind.io>
This feature has no tests and has been broken for ages, see for example
https://github.com/iojs/io.js/pull/1160. Don't bother fixing it, it's
pretty much broken by design and there can't be too many users because
it's almost undocumented. A quick Google search suggests that it causes
more grief than joy to the few that do use it. Remove it.
PR-URL: https://github.com/iojs/io.js/pull/1162
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The NativeModule system passes NativeModule.require transparently and so
is unnecessary to call explicitly.
The only one which should have the prefix is the in line 295, where
actually implements a big fs-based module system and actually requires a
native module. That is left unchanged.
PR-URL: https://github.com/joyent/node/pull/9201
Ref: https://github.com/joyent/node/issues/2009
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Conflicts:
lib/module.js
Many of the util.is*() methods used to check data types
simply compare against a single value or the result of
typeof. This commit replaces calls to these methods with
equivalent checks. This commit does not touch calls to the
more complex methods (isRegExp(), isDate(), etc.).
Fixes: https://github.com/iojs/io.js/issues/607
PR-URL: https://github.com/iojs/io.js/pull/647
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit replaces a number of var statements throughout
the lib code with const statements.
PR-URL: https://github.com/iojs/io.js/pull/541
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
Fixes regression in require system that prevented loading relative
packages via main property in package.json where the file is not
index.{ext}. The regression was introduced in commit 36777d2.
PR-URL: https://github.com/iojs/io.js/pull/145
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Remove a speed bump from commit 36777d2 by reusing the result of the
previous stat() system call. It's a code path that gets called many
thousands of times at startup in most applications so shaving off an
extra system call can have an appreciable impact on startup times.
PR-URL: https://github.com/iojs/io.js/pull/130
Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
Given my home-directory is `/Users/rocko` - and I have a file named
`npm.json` in it and also a repository with name `npm`, which is a
folder for the node-module.
When try to require the `/Users/rocko/npm/index.js` two direcotry
levels down in the npm folder (e.g. `/Users/rocko/npm/test/tap`)
with require("../../") node will load `/Users/rocko/npm/index.json`.
When I use require("../..") node will load `/Users/rocko/npm.json`
which is fixed by this commit.
PR-URL: https://github.com/iojs/io.js/pull/58
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Turn on strict mode for the files in the lib/ directory. It helps
catch bugs and can have a positive effect on performance.
PR-URL: https://github.com/node-forward/node/pull/64
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
this puts the type-checking assertions in require
into proper order.
PR-URL: https://github.com/joyent/node/pull/8333
Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-by: Fedor Indutny <fedor@indutny.com>
Passing a filename is still supported in place of certain options
arguments, for backward-compatibility, but timeout and display-errors
are not translated since those were undocumented.
Also managed to eliminate an extra stack trace line by not calling
through the `createScript` export.
Added a few message tests to show how `displayErrors` works.
As documented in #3042 and in [1], the existing vm implementation has
many problems. All of these are solved by @brianmcd's [contextify][2]
package. This commit uses contextify as a conceptual base and its code
core to overhaul the vm module and fix its many edge cases and caveats.
Functionally, this fixes#3042. In particular:
- A context is now indistinguishable from the object it is based on
(the "sandbox"). A context is simply a sandbox that has been marked
by the vm module, via `vm.createContext`, with special internal
information that allows scripts to be run inside of it.
- Consequently, items added to the context from anywhere are
immediately visible to all code that can access that context, both
inside and outside the virtual machine.
This commit also smooths over the API very slightly:
- Parameter defaults are now uniformly triggered via `undefined`, per
ES6 semantics and previous discussion at [3].
- Several undocumented and problematic features have been removed, e.g.
the conflation of `vm.Script` with `vm` itself, and the fact that
`Script` instances also had all static `vm` methods. The API is now
exactly as documented (although arguably the existence of the
`vm.Script` export is not yet documented, just the `Script` class
itself).
In terms of implementation, this replaces node_script.cc with
node_contextify.cc, which is derived originally from [4] (see [5]) but
has since undergone extensive modifications and iterations to expose
the most useful C++ API and use the coding conventions and utilities of
Node core.
The bindings exposed by `process.binding('contextify')`
(node_contextify.cc) replace those formerly exposed by
`process.binding('evals')` (node_script.cc). They are:
- ContextifyScript(code, [filename]), with methods:
- runInThisContext()
- runInContext(sandbox, [timeout])
- makeContext(sandbox)
From this, the vm.js file builds the entire documented vm module API.
node.js and module.js were modified to use this new native binding, or
the vm module itself where possible. This introduces an extra line or
two into the stack traces of module compilation (and thus into most
stack traces), explaining the changed tests.
The tests were also updated slightly, with all vm-related simple tests
consolidated as test/simple/test-vm-* (some of them were formerly
test/simple/test-script-*). At the same time they switched from
`common.debug` to `console.error` and were updated to use
`assert.throws` instead of rolling their own error-testing methods.
New tests were also added, of course, demonstrating the new
capabilities and fixes.
[1]: http://nodejs.org/docs/v0.10.16/api/vm.html#vm_caveats
[2]: https://github.com/brianmcd/contextify
[3]: https://github.com/joyent/node/issues/5323#issuecomment-20250726
[4]: bf123f3ef9/src/contextify.cc
[5]: https://gist.github.com/domenic/6068120
Add a watchdog class which executes a timer in a separate event loop in
a separate thread that will terminate v8 execution if it expires.
Add timeout argument to functions in vm module which use the watchdog
if a non-zero timeout is specified.
Not necessary, since we can handle the error properly on the first tick
now, even if there are event listeners, etc.
Additionally, this removes the unnecessary "_needTickCallback" from
startup, since Module.loadMain() will kick off a nextTick callback right
after it runs the main module.
Fix#4856
mainly to allow native addons to export single functions on
rather than being restricted to operating on an existing
object.
Init functions now receive exports as the first argument, like
before, but also the module object as the second argument, if they
support it.
Related to #4634
cc: @rvagg
So instead of:
node.js:201
throw e; // process.nextTick error, or 'error' event on first tick
^
You will now see:
path/to/foo.js:1
throw new Error('bar');
^
This is a sub-set of isaacs patch here:
https://github.com/joyent/node/issues/3235
The difference is that this patch purely adresses the exception output,
but does not try to make any behavior changes / improvements.
The current behaviour will silently ignore any parsing errors
that may occur when loading a package.json file. This makes
debugging errors in the package.json file very difficult.
This changes the behaviour that that errors opening and reading
the file package.json file continue to be ignored, but errors
in parsing will throw an exception.
If hasOwnProperty is overridden, then calling `obj.hasOwnProperty(prop)`
can fail. Any time a dictionary of user-generated items is built, we
cannot rely on hasOwnProperty being safe, so must call it from the
Object.prototype explicitly.
Module.globalPaths is still set to a read-only copy of the global
include paths pulled off of the NODE_PATH environment variable.
It's important to be able to inspect this, but modifying it no longer
has any effect.
