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

Core module constants is not documented #8533

Closed
julien-f opened this issue Oct 9, 2014 · 14 comments
Closed

Core module constants is not documented #8533

julien-f opened this issue Oct 9, 2014 · 14 comments

Comments

@julien-f
Copy link

julien-f commented Oct 9, 2014

No description provided.

@trevnorris trevnorris added the doc label Oct 9, 2014
@sam-github
Copy link

Its for node internal use, why would you like it to be documented?

@indutny
Copy link
Member

indutny commented Oct 9, 2014

@sam-github it isn't really for only internal use, we have lots of places where the constants could be supplied.

@sam-github
Copy link

I can see that, but actually exposing constants as API is a bit horrid, IMO. I'd love to see a process.signals containing the SIG* from constants, and I can see putting some of the constants on fs, but exposing a giant bag of C defines in a flat module doesn't seem great to me.

And even though you can supply numeric signals, instead of a string, why would you?

What APIs are there in node that would be easier for the user to use if they had this?

I confess to using the constants to validate whether a user-provided string is actually a valid signal name, but that's about it.

@julien-f
Copy link
Author

I have seen it used in @isaacs's graceful-fs and personally I like to use it for fs.chmod() to express the permissions with constants instead of using octal numbers.

I agree that it may be too low level to expose everything and it would be better to define the most useful constants directly in the appropriate modules.

Nevertheless, because core modules take precedence over those installed in node_modules/ in require(), I believe they should all be documented, at least to express the fact that they exist and are for internal use only. It would (hopefully) avoid issues like the one described in this package.

@brendanashworth
Copy link

If all the modules were documented, I think we'd have a lot of confusing questions and issues regarding modules people shouldn't have touched in the first place - there are certainly modules for standard use and definitely not for standard use. Sometimes, circumstances may render the usage of such an internal module, but I think that you should only (maybe) be using that module in the first place if you know to browse the code in lib/[module].js.

@sam-github
Copy link

I think documenting reserved module names is a good thing, not argument there.

In graceful-fs, and most other uses, typeing out constants.O_RDONLY is longer than 'O_RDONLY' (and uses undocumented interfaces to node...). It would be justified if accessing an undefined property was an error in js, so you could avoid bugs like fs.chmod('README.md', constants.O_RDONLY) (in previous, mode will get set to 0, oops), but it doesn't work like that in js, so I was completely unconvinced.

Except, for file modes, you are right, it does start to make sense, you often have to | them together, so you want the numeric values, because you can't do "S_IRUSR"|"S_IRGRP".

I suggest that all the mode constants be made properties on the fs.Stats function/"class", perhaps on fs.modes. Note that http://nodejs.org/api/fs.html#fs_class_fs_stats shows a mode in its example that is undocumented as to its meaning, and that doesn't have any short-cut methods to analyze it, so it really does need the constants exposed to be meaningful.

@julien-f
Copy link
Author

@sam-github it makes sense.

@brendanashworth if a module is internal and should not be exposed maybe its name should start with an underscore (e.g. _constants) just like _stream_writable.

@sam-github
Copy link

About _, I'd agree. Though there isn't anywhere in the node docs that says such module names are reserved by node.

@brendanashworth
Copy link

It'd be a breaking change to rename all undocumented modules to start with an underscore, but that seems like the most (if not only) logical choice here.

Maybe it would be an option to document in-code instead of on the website (doc folder).

@jasnell
Copy link
Member

jasnell commented Jun 3, 2015

@joyent/node-coreteam ... I doubt this is something we'd ever land here. Recommend deferring to converged.

@trevnorris
Copy link

+1

@orangemocha
Copy link
Contributor

I think there might be some portability issues that are lurking in the constants module. I have some old notes on the subject:

Libuv maps system errors to UV_* error codes. These can have different values depending on the platform (uv-errno.h).
However, constants (from Node_constants.cc) and errno_string() (from Node.cc) define values that correspond to the system headers version of things (i.e. ENOENT). Some of these are defined on both on Unix and Windows (and again they can be different across the two).
In general, these codes can be different (and they are on Windows). For example constants.ENOENT != UV_ENOENT.

So I am -1 on documenting this module before reviewing these issues.

@jasnell
Copy link
Member

jasnell commented Jun 24, 2015

Ok, going to close this issue then. We can revisit this later after convergence.

@jasnell jasnell closed this as completed Jun 24, 2015
@Dzenly
Copy link

Dzenly commented Jan 30, 2016

Just for Info.

Public documentation:
https://nodejs.org/docs/latest-v0.12.x/api/tls.html

Node.js (since v0.10.33) explicitly disables the use of SSLv3 and SSLv2 by setting the secureOptions to be SSL_OP_NO_SSLv3|SSL_OP_NO_SSLv2 (again, unless you have passed --enable-ssl3, or --enable-ssl2, or SSLv3_method as secureProtocol).
...
secureOptions: Set server options. For example, to disable the SSLv3 protocol set the SSL_OP_NO_SSLv3 flag. See SSL_CTX_set_options for all available options.

According to this public API documentation, I now use
secureOptions: constants.SSL_OP_NO_SSLv3 | constants.SSL_OP_NO_SSLv2 | constants.SSL_OP_NO_TLSv1 | constants.SSL_OP_NO_TLSv1_1,
in my code.

There is no support for 'secureOption' in new API documentation, and no mention about these constants though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants