Skip to content

Commit

Permalink
module: speed up package.json parsing
Browse files Browse the repository at this point in the history
If the package.json does not contain the string '"main"', skip parsing
it to JSON.

Note that this changes the behavior of the module loader in the presence
of package.json files that don't contain legal JSON.  Such files used to
throw an exception but now they are simply ignored unless they contain a
"main" property.

To me, that seems like a good trade-off: I observe a 25% reduction in
start-up time on a medium-sized application[0].

[0] https://github.com/strongloop/sls-sample-app

PR-URL: nodejs#15767
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis committed Nov 15, 2017
1 parent 1132ea7 commit fdbb6dd
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 10 deletions.
3 changes: 3 additions & 0 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ function readPackage(requestPath) {
return false;
}

if (!/"main"/.test(json))
return packageMainCache[requestPath] = undefined;

try {
var pkg = packageMainCache[requestPath] = JSON.parse(json).main;
} catch (e) {
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/packages/invalid/index.js

This file was deleted.

1 change: 0 additions & 1 deletion test/fixtures/packages/invalid/package.json

This file was deleted.

8 changes: 0 additions & 8 deletions test/sequential/test-module-loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,6 @@ const d2 = require('../fixtures/b/d');
assert.notStrictEqual(threeFolder, three);
}

console.error('test package.json require() loading');
assert.throws(
function() {
require('../fixtures/packages/invalid');
},
/^SyntaxError: Error parsing .+: Unexpected token , in JSON at position 1$/
);

assert.strictEqual(require('../fixtures/packages/index').ok, 'ok',
'Failed loading package');
assert.strictEqual(require('../fixtures/packages/main').ok, 'ok',
Expand Down

0 comments on commit fdbb6dd

Please sign in to comment.