This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
Cleanup of 'NativeModule.require' calls in lib/module.js #2009
Labels
Comments
|
please provide a patch |
|
https://gist.github.com/1340039 |
|
Hello. Was the patch somehow inadequate or my answer only slipped somehow? @ry |
|
Hello! I sent the patch as you requested. Is any other problem with this issue? |
|
@Herby: Can you submit it as a pull request? Thanks. |
|
Is it possible to reopen this? I can then issue a PR that fixes it. |
|
Ping @isaacs. |
ghost
mentioned this issue
trevnorris
pushed a commit
that referenced
this issue
Feb 12, 2015
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: #9201 Ref: #2009 Reviewed-by: Trevor Norris <trev.norris@gmail.com>
cjihrig
pushed a commit
to nodejs/node
that referenced
this issue
Feb 13, 2015
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: nodejs/node-v0.x-archive#9201 Ref: nodejs/node-v0.x-archive#2009 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Conflicts: lib/module.js
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
All calls to
NativeModule.requirecalled with string literal argument ('fs'and'path') are unnecessarily explicitly referring toNativeModuleand can and should be changed to plainrequirecall. This matches with the way how any other native module perform these requires (module.js is in no way special in this).Only call which should retain the explicit
NativeModule.requireis the one insideModule._loadwhich calls with a variable argumentid, not with a literal argument.The text was updated successfully, but these errors were encountered: