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

doc: createRequire() improvements #27762

Merged
merged 2 commits into from
May 20, 2019
Merged

doc: createRequire() improvements #27762

merged 2 commits into from
May 20, 2019

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 18, 2019

First commit:
Update the createRequire() example to use import and import.meta.url instead of require() and require.resolve().

I'm not so sure about the change here to .eslintrc.js. Based on eslint/eslint#8148, it doesn't seem possible to configure the parserOptions.sourceType per code block. If it is possible, I'm happy to make the change. Another option would be to just disable eslint for that code block.

Second commit:
This commit replaces createRequireFromPath() references with createRequire() references.

Fixes: #27758

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label May 18, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 20, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/23231/

EDIT(cjihrig): CI was yellow.

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>
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>
@cjihrig
Copy link
Contributor Author

cjihrig commented May 20, 2019

Landed in 64182e9...1b381d6.

@cjihrig cjihrig closed this May 20, 2019
@cjihrig cjihrig deleted the doc branch May 20, 2019 14:24
@cjihrig cjihrig merged commit 1b381d6 into nodejs:master May 20, 2019
BridgeAR pushed a commit that referenced this pull request 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 pull request 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>
@BridgeAR BridgeAR mentioned this pull request May 21, 2019
4 tasks
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request 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
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix documentation for the new createRequire()
4 participants