Feat: add support for NODE_DEBUG=module #96
Conversation
|
Tagging @ganigeorgiev for input and discussion if needed. |
|
@benallfree Please stop spamming me! I have no context about why the suggested change is even needed and I'm not the maintainer of the |
require/resolve.go
Outdated
| p := path.Join(start, modpath) | ||
| if isFileOrDirectoryPath(origPath) { | ||
| if module = r.modules[p]; module != nil { | ||
| moduleDebug(fmt.Sprintf("resolve %s (cached)", p)) |
There was a problem hiding this comment.
It's not the most efficient way of doing it. The fmt.Sprintf() should not be called if debug is disabled.
require/resolve.go
Outdated
| } | ||
|
|
||
| func moduleDebug(msg string) { | ||
| if os.Getenv("NODE_DEBUG") == "module" { |
There was a problem hiding this comment.
os.Getenv will do a syscall every time. Again, not the most efficient way. It should be cached once and then re-used. I think node works this way.
There was a problem hiding this comment.
Also, NODE_DEBUG may contain comma-separated values.
|
I've added support for comma-separated module names and made the optimization improvements you suggested. Let me know what you think :) |
|
@dop251 I realized a while ago that my method of finding the module name from the module path is probably not accurate because a user could do something like |
* Added variable length `Buffer.read*` methods - added `readIntBE`, `readIntLE`, `readUIntBE`, and `readUIntLE`. - also refactored out some common methods to create errors * added errors.NewRangeError. Also fixed mispelled `ErrCodedOutOfRange` -> `ErrCodeOutOfRange` (extra `d`) * added the `readUint*` (small `i`) aliases --------- Co-authored-by: dave sinclair <david_sinclair@cable.comcast.com>
* Added write method - added the `write` method and fleshed out the `toString` method to match the behavior with Node's `Buffer` class. The test cases are pretty exhaustive to ensure all the edge cases, and Node specific behavior is covered. * Use getRequiredStringArgument
* Added writeBig* methods - added `writeBigInt64BE`, `writeBigInt64LE`, `writeBigUInt64BE`, and `writeBigUInt64LE` along with their aliases * Simplified tests - added the `assertions.js` file for the buffer tests - refactored the `assert.throws*` method to reduce the duplicate code
…sed by other modules.
…hout host or path.
…link resolution. Most path operations are now done in the OS semantics (i.e. using filepath/path). (dop251#101)
- added the rest of the `Buffer.write*` methods. Also updated the `assert._isSameValue` to handle number specifically to handle floating point differences that are within a precision.
|
@dop251 Just sync'd with master. How do you feel about this PR? It has been very useful in local testing for debugging module resolution. |
|
I'm not sure this is implemented in line with how it's working in node. The syntax is defined as |
|
As I understand it, the presence of Do you agree? |
|
I think it's just "a,b,c,..." and "a" in this case is "module". So the "module" in the documentation is not a literal, it's a name of a module, which in our case happens to be "module". |
|
🤣 my bad I'll fix. I always thought |
4 participants
This PR mimics NODE_DEBUG=module by outputting module resolution debugging information.
Example:
Outputs...