FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

esm: share package.json cache between ESM and CJS loaders · nodejs/node@3559471 · GitHub

/ node Public

Commit 3559471

Browse files
shackijj
authored and
codebytere
committed
esm: share package.json cache between ESM and CJS loaders
Refs: #30674 PR-URL: #33229 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
1 parent 10b88cb commit 3559471

10 files changed

Lines changed: 119 additions & 67 deletions

File tree

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,8 @@ const assert = require('internal/assert');
5555
const fs = require('fs');
5656
const internalFS = require('internal/fs/utils');
5757
const path = require('path');
58-
const {
59-
internalModuleReadJSON,
60-
internalModuleStat
61-
} = internalBinding('fs');
58+
const { internalModuleStat } = internalBinding('fs');
59+
const packageJsonReader = require('internal/modules/package_json_reader');
6260
const { safeGetenv } = internalBinding('credentials');
6361
const {
6462
makeRequireFunction,
@@ -248,7 +246,8 @@ function readPackage(requestPath) {
248246
const existing = packageJsonCache.get(jsonPath);
249247
if (existing !== undefined) return existing;
250248

251-
const json = internalModuleReadJSON(path.toNamespacedPath(jsonPath));
249+
const result = packageJsonReader.read(path.toNamespacedPath(jsonPath));
250+
const json = result.containsKeys === false ? '{}' : result.string;
252251
if (json === undefined) {
253252
packageJsonCache.set(jsonPath, false);
254253
return false;

‎lib/internal/modules/esm/resolve.js‎

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ const assert = require('internal/assert');
2626
const internalFS = require('internal/fs/utils');
2727
const { NativeModule } = require('internal/bootstrap/loaders');
2828
const {
29-
closeSync,
30-
fstatSync,
31-
openSync,
32-
readFileSync,
3329
realpathSync,
3430
statSync,
3531
Stats,
@@ -53,6 +49,7 @@ const {
5349
ERR_UNSUPPORTED_ESM_URL_SCHEME,
5450
} = require('internal/errors').codes;
5551

52+
const packageJsonReader = require('internal/modules/package_json_reader');
5653
const DEFAULT_CONDITIONS = ObjectFreeze(['node', 'import']);
5754
const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS);
5855

@@ -78,37 +75,25 @@ function tryStatSync(path) {
7875
}
7976
}
8077

81-
function readIfFile(path) {
82-
let fd;
83-
try {
84-
fd = openSync(path, 'r');
85-
} catch {
86-
return undefined;
87-
}
88-
try {
89-
if (!fstatSync(fd).isFile()) return undefined;
90-
return readFileSync(fd, 'utf8');
91-
} finally {
92-
closeSync(fd);
93-
}
78+
/**
79+
*
80+
* '/foo/package.json' -> '/foo'
81+
*/
82+
function removePackageJsonFromPath(path) {
83+
return StringPrototypeSlice(path, 0, path.length - 13);
9484
}
9585

96-
function getPackageConfig(path, base) {
86+
function getPackageConfig(path) {
9787
const existing = packageJSONCache.get(path);
9888
if (existing !== undefined) {
99-
if (!existing.isValid) {
100-
throw new ERR_INVALID_PACKAGE_CONFIG(path, fileURLToPath(base), false);
101-
}
10289
return existing;
10390
}
104-
105-
const source = readIfFile(path);
91+
const source = packageJsonReader.read(path).string;
10692
if (source === undefined) {
10793
const packageConfig = {
10894
exists: false,
10995
main: undefined,
11096
name: undefined,
111-
isValid: true,
11297
type: 'none',
11398
exports: undefined
11499
};
@@ -119,17 +104,9 @@ function getPackageConfig(path, base) {
119104
let packageJSON;
120105
try {
121106
packageJSON = JSONParse(source);
122-
} catch {
123-
const packageConfig = {
124-
exists: true,
125-
main: undefined,
126-
name: undefined,
127-
isValid: false,
128-
type: 'none',
129-
exports: undefined
130-
};
131-
packageJSONCache.set(path, packageConfig);
132-
return packageConfig;
107+
} catch (error) {
108+
const errorPath = removePackageJsonFromPath(path);
109+
throw new ERR_INVALID_PACKAGE_CONFIG(errorPath, error.message, true);
133110
}
134111

135112
let { main, name, type } = packageJSON;
@@ -143,7 +120,6 @@ function getPackageConfig(path, base) {
143120
exists: true,
144121
main,
145122
name,
146-
isValid: true,
147123
type,
148124
exports
149125
};
@@ -171,7 +147,6 @@ function getPackageScopeConfig(resolved, base) {
171147
exists: false,
172148
main: undefined,
173149
name: undefined,
174-
isValid: true,
175150
type: 'none',
176151
exports: undefined
177152
};
@@ -590,8 +565,7 @@ function packageResolve(specifier, base, conditions) {
590565
let packageJSONPath = fileURLToPath(packageJSONUrl);
591566
let lastPath;
592567
do {
593-
const stat = tryStatSync(
594-
StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13));
568+
const stat = tryStatSync(removePackageJsonFromPath(packageJSONPath));
595569
if (!stat.isDirectory()) {
596570
lastPath = packageJSONPath;
597571
packageJSONUrl = new URL((isScoped ?

‎lib/internal/modules/package_json_reader.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
3+
const { SafeMap } = primordials;
4+
const { internalModuleReadJSON } = internalBinding('fs');
5+
6+
const cache = new SafeMap();
7+
8+
/**
9+
*
10+
* @param {string} path
11+
*/
12+
function read(path) {
13+
if (cache.has(path)) {
14+
return cache.get(path);
15+
}
16+
17+
const [string, containsKeys] = internalModuleReadJSON(path);
18+
const result = { string, containsKeys };
19+
cache.set(path, result);
20+
return result;
21+
}
22+
23+
module.exports = { read };

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@
155155
'lib/internal/main/run_third_party_main.js',
156156
'lib/internal/main/worker_thread.js',
157157
'lib/internal/modules/run_main.js',
158+
'lib/internal/modules/package_json_reader.js',
158159
'lib/internal/modules/cjs/helpers.js',
159160
'lib/internal/modules/cjs/loader.js',
160161
'lib/internal/modules/esm/loader.js',

‎src/node_file.cc‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ namespace node {
5151
namespace fs {
5252

5353
using v8::Array;
54+
using v8::Boolean;
5455
using v8::Context;
5556
using v8::EscapableHandleScope;
5657
using v8::Function;
@@ -826,9 +827,7 @@ void Close(const FunctionCallbackInfo<Value>& args) {
826827
}
827828

828829

829-
// Used to speed up module loading. Returns the contents of the file as
830-
// a string or undefined when the file cannot be opened or "main" is not found
831-
// in the file.
830+
// Used to speed up module loading. Returns an array [string, boolean]
832831
static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) {
833832
Environment* env = Environment::GetCurrent(args);
834833
Isolate* isolate = env->isolate();
@@ -837,14 +836,16 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) {
837836
CHECK(args[0]->IsString());
838837
node::Utf8Value path(isolate, args[0]);
839838

840-
if (strlen(*path) != path.length())
839+
if (strlen(*path) != path.length()) {
840+
args.GetReturnValue().Set(Array::New(isolate));
841841
return; // Contains a nul byte.
842-
842+
}
843843
uv_fs_t open_req;
844844
const int fd = uv_fs_open(loop, &open_req, *path, O_RDONLY, 0, nullptr);
845845
uv_fs_req_cleanup(&open_req);
846846

847847
if (fd < 0) {
848+
args.GetReturnValue().Set(Array::New(isolate));
848849
return;
849850
}
850851

@@ -870,9 +871,10 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) {
870871
numchars = uv_fs_read(loop, &read_req, fd, &buf, 1, offset, nullptr);
871872
uv_fs_req_cleanup(&read_req);
872873

873-
if (numchars < 0)
874+
if (numchars < 0) {
875+
args.GetReturnValue().Set(Array::New(isolate));
874876
return;
875-
877+
}
876878
offset += numchars;
877879
} while (static_cast<size_t>(numchars) == kBlockSize);
878880

@@ -908,18 +910,16 @@ static void InternalModuleReadJSON(const FunctionCallbackInfo<Value>& args) {
908910
}
909911
}
910912

911-
Local<String> return_value;
912-
if (p < pe) {
913-
return_value =
914-
String::NewFromUtf8(isolate,
915-
&chars[start],
916-
v8::NewStringType::kNormal,
917-
size).ToLocalChecked();
918-
} else {
919-
return_value = env->empty_object_string();
920-
}
921913

922-
args.GetReturnValue().Set(return_value);
914+
Local<Value> return_value[] = {
915+
String::NewFromUtf8(isolate,
916+
&chars[start],
917+
v8::NewStringType::kNormal,
918+
size).ToLocalChecked(),
919+
Boolean::New(isolate, p < pe ? true : false)
920+
};
921+
args.GetReturnValue().Set(
922+
Array::New(isolate, return_value, arraysize(return_value)));
923923
}
924924

925925
// Used to speed up module loading. Returns 0 if the path refers to

‎test/es-module/test-esm-invalid-pjson.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use strict';
2+
3+
const { mustCall, isWindows } = require('../common');
4+
const fixtures = require('../common/fixtures');
5+
const { spawn } = require('child_process');
6+
const { strictEqual, ok } = require('assert');
7+
8+
const entry = fixtures.path('/es-modules/import-invalid-pjson.mjs');
9+
const invalidJson = fixtures.path('/node_modules/invalid-pjson/package.json');
10+
11+
const child = spawn(process.execPath, [entry]);
12+
child.stderr.setEncoding('utf8');
13+
let stderr = '';
14+
child.stderr.on('data', (data) => {
15+
stderr += data;
16+
});
17+
child.on('close', mustCall((code, signal) => {
18+
strictEqual(code, 1);
19+
strictEqual(signal, null);
20+
ok(
21+
stderr.includes(
22+
[
23+
'[ERR_INVALID_PACKAGE_CONFIG]: ',
24+
`Invalid package config ${invalidJson}, `,
25+
`Unexpected token } in JSON at position ${isWindows ? 16 : 14}`
26+
].join(''),
27+
),
28+
stderr);
29+
}));

‎test/fixtures/es-modules/import-invalid-pjson.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import 'invalid-pjson';

‎test/fixtures/node_modules/invalid-pjson/package.json‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/parallel/test-bootstrap-modules.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const expectedModules = new Set([
4949
'NativeModule internal/idna',
5050
'NativeModule internal/linkedlist',
5151
'NativeModule internal/modules/run_main',
52+
'NativeModule internal/modules/package_json_reader',
5253
'NativeModule internal/modules/cjs/helpers',
5354
'NativeModule internal/modules/cjs/loader',
5455
'NativeModule internal/modules/esm/create_dynamic_module',

‎test/parallel/test-module-binding.js‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,32 @@ const { internalBinding } = require('internal/test/binding');
66
const { internalModuleReadJSON } = internalBinding('fs');
77
const { readFileSync } = require('fs');
88
const { strictEqual } = require('assert');
9-
10-
strictEqual(internalModuleReadJSON('nosuchfile'), undefined);
11-
strictEqual(internalModuleReadJSON(fixtures.path('empty.txt')), '{}');
12-
strictEqual(internalModuleReadJSON(fixtures.path('empty-with-bom.txt')), '{}');
9+
{
10+
const [string, containsKeys] = internalModuleReadJSON('nosuchfile');
11+
strictEqual(string, undefined);
12+
strictEqual(containsKeys, undefined);
13+
}
14+
{
15+
const [string, containsKeys] =
16+
internalModuleReadJSON(fixtures.path('empty.txt'));
17+
strictEqual(string, '');
18+
strictEqual(containsKeys, false);
19+
}
20+
{
21+
const [string, containsKeys] =
22+
internalModuleReadJSON(fixtures.path('empty.txt'));
23+
strictEqual(string, '');
24+
strictEqual(containsKeys, false);
25+
}
26+
{
27+
const [string, containsKeys] =
28+
internalModuleReadJSON(fixtures.path('empty-with-bom.txt'));
29+
strictEqual(string, '');
30+
strictEqual(containsKeys, false);
31+
}
1332
{
1433
const filename = fixtures.path('require-bin/package.json');
15-
strictEqual(internalModuleReadJSON(filename), readFileSync(filename, 'utf8'));
34+
const [string, containsKeys] = internalModuleReadJSON(filename);
35+
strictEqual(string, readFileSync(filename, 'utf8'));
36+
strictEqual(containsKeys, true);
1637
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL