Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

In all native modules, require should be called. #9201

Closed
wants to merge 1 commit into from
Closed

In all native modules, require should be called. #9201

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 12, 2015

The NativeModule system passes NativeModule.require transparently there. All other require calls in this module are without NativeModule. prefix, these three are last to finally finish what should be done back there when lib/module.js was being extracted.

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.

The NativeModule system passes `NativeModule.require` transparently there. All other require calls in this module are without `NativeModule.` prefix, these three are last to finally finish what should be done back there when lib/module.js was being extracted.

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.
@ghost
Copy link
Author

ghost commented Feb 12, 2015

Fixes #2009.

trevnorris pushed a commit that referenced this pull request 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 <[email protected]>
@trevnorris
Copy link

Thanks. Landed in b8604fa with minor commit message reword.

@trevnorris trevnorris closed this Feb 12, 2015
@ghost ghost deleted the patch-1 branch February 12, 2015 23:17
cjihrig pushed a commit to nodejs/node that referenced this pull request 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 <[email protected]>

Conflicts:
	lib/module.js
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants