Clarify behavior of require('http') and require('node:http') with existing require.cache entries #52992
Conversation
…sting require.cache entries
nodejs-github-bot
added
doc
|
@nodejs/documentation |
| passed to `require()`. For instance, `require('http')` will | ||
| return the built-in HTTP module, even if there is a file by that name. However, | ||
| note that core modules specified without the `node:` prefix are served from the | ||
| cache if an entry for the module exists. Because cache entries can be manually | ||
| altered by modifying the `require.cache `object, this allows the module's | ||
| implementation to be replaced. To ensure `require() ` returns the original | ||
| implementation, use the `node:` prefix. The list |
There was a problem hiding this comment.
I would be more direct, at the risk of being vague: mutation of the module cache is a thing in CJS, but that shouldn't matter for most users – and for those for which it does matter, it seems fair to assume they won't need the extra details.
| passed to `require()`. For instance, `require('http')` will | |
| return the built-in HTTP module, even if there is a file by that name. However, | |
| note that core modules specified without the `node:` prefix are served from the | |
| cache if an entry for the module exists. Because cache entries can be manually | |
| altered by modifying the `require.cache `object, this allows the module's | |
| implementation to be replaced. To ensure `require() ` returns the original | |
| implementation, use the `node:` prefix. The list | |
| passed to `require()`. For instance, assuming no mutation of the module cache, `require('http')` | |
| will return the built-in HTTP module, even if there is a file by that name. The list |
There was a problem hiding this comment.
As a newcomer to this, I find the wording "assuming no mutation of the module cache" somewhat confusing.
My goal is to clearly understand the specification, and this phrase doesn't help clarify the situation for me.
I believe my initial wording is preferable because it explicitly states the technical condition under which the behavior differs.
There was a problem hiding this comment.
How is it confusing, and how can we clear up that confusion?
There was a problem hiding this comment.
I find the wording "assuming no mutation of the module cache" somewhat confusing because it implies that mutations exist, which begs the question: "under what circumstances?" My wording describes the specification as it is. As for how to move forward, feel free to decide how to write this.
There was a problem hiding this comment.
I think that's fine, the rest of the documentation should contain the answer to that question. Maybe it's clearer if we say require.cache instead of "the module cache"
| passed to `require()`. For instance, `require('http')` will | |
| return the built-in HTTP module, even if there is a file by that name. However, | |
| note that core modules specified without the `node:` prefix are served from the | |
| cache if an entry for the module exists. Because cache entries can be manually | |
| altered by modifying the `require.cache `object, this allows the module's | |
| implementation to be replaced. To ensure `require() ` returns the original | |
| implementation, use the `node:` prefix. The list | |
| passed to `require()`. For instance, assuming `require.cache` is not modified, `require('http')` | |
| will return the built-in HTTP module, even if there is a file by that name. The list |
If you look at the section just above this one, it already deals with how caching works in CJS
There was a problem hiding this comment.
IMO it should be past tense, 'assuming require.cache has not been modified'
There was a problem hiding this comment.
FYI, I initially raised this question in the IRC, and the reply I got indicated that there is an impression that core modules are not served from the cache at all, which suggests that this is not so clear. Just my 2c.
|
@aduh95 prefers different wording, so this PR is being closed. |
To clarify, my comments are non-blocking. If you re-open, that might give opportunity for more people to contribute to the discussion. |
|
@aduh95 I apologize, but this is taking up more of my attention than I can afford. I believe the amendment is trivial, and I see no issue with my wording. It's clear that we have different writing styles, which is okay. I don't have a particular interest in advocating for my style. I think we all agree on the overall message, and the GitHub discussion is still open. |
I'm sorry to hear that, if you'd like, someone could make some changes, and credit you as the (I'm speaking for myself, and not on behalf of the 'someone') |
|
@RedYetiDev No worries! No credit needed. I'm happy to be the behind-the-scenes inspiration. 😄 |
No description provided.