Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix documentation for the new createRequire() #27758

Closed
rauschma opened this issue May 18, 2019 · 1 comment · Fixed by #27762
Closed

Fix documentation for the new createRequire() #27758

rauschma opened this issue May 18, 2019 · 1 comment · Fixed by #27762
Labels
doc Issues and PRs related to the documentations.

Comments

@rauschma
Copy link

First issue: https://nodejs.org/dist/v12.2.0/docs/api/esm.html

Problem: mentions the deprecated module.createRequireFromPath() (and not module.createRequire()).

Second issue: https://nodejs.org/dist/v12.2.0/docs/api/modules.html#modules_module_createrequire_filename

The code example is:

const { createRequire } = require('module');
const requireUtil = createRequire(require.resolve('../src/utils/'));

// Require `../src/utils/some-tool`
requireUtil('./some-tool');

I believe this would be a better example (import vs. require; import.meta.url vs. require.resolve()):

import {createRequire} from 'module';
const require = createRequire(import.meta.url);

const siblingModule = require('./sibling-module.cjs');

Additionally, one could demonstrate url.pathToFileURL() and import a module via an absolute path. I suspect that it may be better not to allow absolute paths as arguments for createRequire().

@guybedford
Copy link
Contributor

@rauschma thanks so much for pointing these out! I agree both of the suggestions sound like documentation corrections that we should add.

@guybedford guybedford added the doc Issues and PRs related to the documentations. label May 18, 2019
cjihrig added a commit to cjihrig/node that referenced this issue May 20, 2019
Update the example to use import and import.meta.url instead
of require() and require.resolve().

PR-URL: nodejs#27762
Fixes: nodejs#27758
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
cjihrig added a commit to cjihrig/node that referenced this issue May 20, 2019
This commit replaces createRequireFromPath() references with
createRequire() references.

PR-URL: nodejs#27762
Fixes: nodejs#27758
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
BridgeAR pushed a commit that referenced this issue May 21, 2019
Update the example to use import and import.meta.url instead
of require() and require.resolve().

PR-URL: #27762
Fixes: #27758
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
BridgeAR pushed a commit that referenced this issue May 21, 2019
This commit replaces createRequireFromPath() references with
createRequire() references.

PR-URL: #27762
Fixes: #27758
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
Update the example to use import and import.meta.url instead
of require() and require.resolve().

PR-URL: nodejs/node#27762
Fixes: nodejs/node#27758
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants