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

Optimize Node.js require() calls #2262

Open
octogonz opened this issue Oct 7, 2020 · 2 comments
Open

Optimize Node.js require() calls #2262

octogonz opened this issue Oct 7, 2020 · 2 comments
Labels
general discussion Not a bug or enhancement, just a discussion

Comments

@octogonz
Copy link
Collaborator

octogonz commented Oct 7, 2020

In #2254 (comment), @dmichon-msft pointed out that:

I should point out that resolution will, in general, be faster and more robust if you always reference path/to/index, even if you can refer to it by folder in the node resolution algorithm. Folder imports cause the resolver to look for a package.json to try to ascertain the main file.

However our aim is to provide a great developer experience, so it seems a bit awkward to ask users to write this:

import { X } from './folder1/folder2/index';

...instead of this:

import { X } from './folder1/folder2';

Microoptimizations are the job of the tooling, not something humans should have to worry about. Let's discuss:

  • How significant are these extra filesystem accesses, really?
  • If they matter, what are the options for eliminating them?

CC @iclanton @sachinjoseph

@octogonz octogonz added the general discussion Not a bug or enhancement, just a discussion label Oct 7, 2020
@octogonz
Copy link
Collaborator Author

octogonz commented Oct 7, 2020

Here's the output of Process Monitor comparing three require() statements:

  • require("./folder1/folder2/index");
  • require("./folder1/folder2/");
  • require("./folder1/folder2");

Capture

Results

The package.json access seems to happen in all 3 cases, just a bit later.

Simply appending a slash require("./folder1/folder2/"); however does eliminate three read attempts:

C:\test\folder1\folder2.js
C:\test\folder1\folder2.json
C:\test\folder1\folder2.node

And appending /index goes a step further and eliminates a fs.readdir()-type operation for C:\test\folder1\folder2.

@octogonz
Copy link
Collaborator Author

octogonz commented Oct 7, 2020

Here's a some possible options we could consider:

  1. Implement an ESLint rule that asks for path imports to be written as "./folder1/folder2/" with a trailing slash. This isn't as awkward as "./folder1/folder2/index", and maybe still provides most of the benefits.

  2. Implement a compiler transform that automatically normalizes paths to "./folder1/folder2/index" in the emitted .js files. This would be completely transparent to users, the best developer experience. It would also enables

  3. Simply use Webpack to bundle our Node.js projects. This has been suggested many times before and would probably be a huge performance improvements. The classic concern was with bundling node_modules dependencies, which often are incompatible with bundling. Plus there's a license attribution concern when embedding another project's code. But if we entirely externalized the node_modules dependencies, Webpack could still provide some benefits.

(Before proceeding though, someone would need to collect some performance measurements showing that this investment would actually be worthwhile.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general discussion Not a bug or enhancement, just a discussion
Projects
Status: General Discussions
Development

No branches or pull requests

1 participant