nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.41k stars 29.51k forks source link

doc: module.createRequire() not descriptive #40567

Open dnalborczyk opened 3 years ago

dnalborczyk commented 3 years ago

📗 API Reference Docs Problem

Latest (and prior)

n/a

'module'

Location

see below

Description

Concise explanation of the problem

✍️

for reference: https://nodejs.org/dist/latest-v17.x/docs/api/module.html#modulecreaterequirefilename

the documentation for createRequire for the filename parameter states:

Filename to be used to construct the require function. Must be a file URL object, file URL string, or absolute path string.

which is not really descriptive about what is expected and how it influences the behavior.

from an older version of the [now removed] createRequireFromPath function the docs state: https://nodejs.org/dist/v12.2.0/docs/api/modules.html#modules_module_createrequirefrompath_filename

Filename to be used to construct the relative require function.

I assume it puts the [created] require function in the relative path (directory) of the specified filename parameter. Not really sure how I would express this in a meaningful way. 🤔

I think it would also make sense to add a little explainer why such a thing exists: require does not exist in an es module context vs. commonjs can also be imported with es module syntax, although the expectations and behavior in both are likely very different as well.

the docs could also state that createRequire is less useful in a commonjs context, e.g. const { createRequire: _require = require('module')


aduh95 commented 3 years ago

the docs could also state that createRequire is less useful in a commonjs context, e.g. const { createRequire: _require = require('module')

I don't think the docs should say anything regarding the usefulness of that API (it's subjective imho, one could argue it's not useful in ESM context either; it's only useful if you have a use case, and there are use cases in both ESM and CJS contexts).

I think it would also make sense to add a little explainer why such a thing exists: require does not exist in an es module context vs. commonjs can also be imported with es module syntax, although the expectations and behavior in both are likely very different as well.

Other notable differences:

We could try to add a list of use cases for this API, it may be difficult to maintain said list though.

which is not really descriptive about what is expected and how it influences the behavior.

It's using the path/URL to resolve relative paths (e.g.: createRequire('/foo/bar')('./baz') may load /foo/baz/index.js). Do you think adding an example would help?

dnalborczyk commented 3 years ago

the docs could also state that createRequire is less useful in a commonjs context, e.g. const { createRequire: _require = require('module')

I don't think the docs should say anything regarding the usefulness of that API (it's subjective imho, one could argue it's not useful in ESM context either; it's only useful if you have a use case, and there are use cases in both ESM and CJS contexts).

generally speaking, I agree with your sentiment, although I think this particular case is a bit different.

what I meant with "less useful": historically this function has been added to support loading commonjs in esm, before it was possible to load it at all, or during the time cjs interop support was added. possibly also as a fallback for things where there wasn't any (official) support in esm (yet): e.g. '.json', '.node'? (not sure), '.cjs'? (haven't tried it yet). I suppose for API consistency between cjs and esm (or technical reasons) createRequire can be required from commonjs as well.

for anyone not knowing about the history, and reading the current docs, it might appear as a weird conundrum that you can require something in order to create the same thing you needed to require it in the first place.`

that's why I assume the docs did change the example from using commonjs syntax (as seen in the older versions) to just using esm instead.

for the above reasons, I think the usage of might be less useful .... sounds suitable to me.

I think it would also make sense to add a little explainer why such a thing exists: require does not exist in an es module context vs. commonjs can also be imported with es module syntax, although the expectations and behavior in both are likely very different as well.

Other notable differences:

  • you can require a directory, you cannot import it.
  • you can require a native module, no support for it in the current ESM implementation.
  • require cache is user mutable, ESM cache is internal only.
  • there might be other things I'm missing.

yeah, I agree, although for those things a pointer to require is probably sufficient if there's no behavioral differences with require. although some things you mentioned were not clear to me right away. for example I was wondering if the module instance is being shared, or the module cache (it's not).

// foo.js (commonjs)
exports.foo = {}

exports.require = require
// index.mjs (esm)
import { foo as a, require as requireImported } from "./foo.js"
import { createRequire } from "module"

const requireCreated = createRequire(import.meta.url)

const { foo: b } = requireImported("./foo.js")
const { foo: c } = requireCreated("./foo.js")
const { foo: d } = await import("./foo.js")

console.log(a === b) // true
console.log(a === c) // true
console.log(a === d) // true
console.log(b === c) // true
console.log(b === d) // true
console.log(c === d) // true
node ./index.mjs

We could try to add a list of use cases for this API, it may be difficult to maintain said list though.

yeah, not sure. I think it wouldn't need to be exhaustive, just some examples maybe?

which is not really descriptive about what is expected and how it influences the behavior.

It's using the path/URL to resolve relative paths (e.g.: createRequire('/foo/bar')('./baz') may load /foo/baz/index.js). Do you think adding an example would help?

yeah, I think some additional explainer and an additional example, possibly even the same or similar as the one for createRequireFromPath linked above.

the current example const require = createRequire(import.meta.url) is great, and I would imagine is being used for 99% of the (esm) use cases, as it puts require into the path of the current file.

Filename to be used to construct the require function. doesn't tell me much what it does or for what it is being used for. other than it's a filename, and some sort of constructor. what should the location be? does it need to exist? what if it doesn't?

btw, this last part was my initiator for this issue, the other things would be just a nice-to-have kinda thing for other users having similar questions.