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

Exposing the logic behind require.resolve() via a public API #16389

Closed
zkochan opened this issue Oct 22, 2017 · 4 comments
Closed

Exposing the logic behind require.resolve() via a public API #16389

zkochan opened this issue Oct 22, 2017 · 4 comments
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.

Comments

@zkochan
Copy link

zkochan commented Oct 22, 2017

Node.js does not expose any public API for resolving dependencies. As a result, there are many packages in the ecosystem that are trying to reimplement/mimic Node's module resolution algorithm.

resolve, resolve-from, eslint-plugin-import... just to list some of them.

The big problem with these implementations is that they don't work the way Node's require.resolve works. For instance, both resolve and resolve-from were preserving symlinks by default. I did PRs to both packages to fix this issue but they are just a drop in the ocean.

I work on pnpm - a Node package manager that uses a symlinked node_modules structure. This node_modules structure is Node.js-compatible (when Node is executed w/o the --preserve-symlinks flag) but because of the many incorrect implementations of Node's module resolution algorithm, pnpm is basically unusable with most of the popular frameworks/toolings.

Exposing Node's resolution algorithm would allow experimenting with different node_modules structures and tools/frameworks would adjust because they would resolve dependencies correctly.

@cjihrig
Copy link
Contributor

cjihrig commented Oct 22, 2017

Related: #5963

@mscdex mscdex added feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem. labels Oct 23, 2017
@zkochan
Copy link
Author

zkochan commented Oct 23, 2017

Seems like the PR by @cjihrig would make it possible to replace the resolve-from by @sindresorhus. However, resolve works asynchronously. Could an async implementation be added to Node as well? I am afraid some tools might prefer to stay with an async implementation.

If the async implementation is too much in node core, maybe it can be in an official module maintained by Node that would work 100% the way sync require.resolve works.

cc @ljharb

P.S. thanks for the quick response!

@bnoordhuis
Copy link
Member

However, resolve works asynchronously.

require.resolve()? It's synchronous.

If "resolve works synchronously" is what you intended to write, just wrap it in process.nextTick() or setImmediate().

@zkochan
Copy link
Author

zkochan commented Oct 23, 2017

I am personally fine with the sync require.resolve. What I mean is that there are implementations of require.resolve in userland which are async (like substack's resolve).

So I wonder if it would be enough to have only a sync require.resolve exposed or some packages would still prefer an async one from userland.

addaleax pushed a commit to ayojs/ayo that referenced this issue Oct 26, 2017
This commit allows custom lookup paths to be passed to
require.resolve(). It also adds require.resolve.paths()
which retrieves the default resolution paths.

Fixes: nodejs/node#5963
Fixes: nodejs/node#16389
PR-URL: nodejs/node#16397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
gibfahn pushed a commit that referenced this issue Oct 30, 2017
This commit allows custom lookup paths to be passed to
require.resolve(). It also adds require.resolve.paths()
which retrieves the default resolution paths.

Fixes: #5963
Fixes: #16389
PR-URL: #16397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
gibfahn pushed a commit that referenced this issue Oct 30, 2017
This commit allows custom lookup paths to be passed to
require.resolve(). It also adds require.resolve.paths()
which retrieves the default resolution paths.

Fixes: #5963
Fixes: #16389
PR-URL: #16397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
gibfahn pushed a commit that referenced this issue Oct 31, 2017
This commit allows custom lookup paths to be passed to
require.resolve(). It also adds require.resolve.paths()
which retrieves the default resolution paths.

Fixes: #5963
Fixes: #16389
PR-URL: #16397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
addaleax pushed a commit to ayojs/ayo that referenced this issue Dec 7, 2017
This commit allows custom lookup paths to be passed to
require.resolve(). It also adds require.resolve.paths()
which retrieves the default resolution paths.

Fixes: nodejs/node#5963
Fixes: nodejs/node#16389
PR-URL: nodejs/node#16397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants