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

Remove isBuiltIn and builtinModules #2662

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/node/internal/module.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@
// https://opensource.org/licenses/Apache-2.0

export function createRequire(path: string): (specifier: string) => unknown;
export function isBuiltin(specifier: string): boolean;
88 changes: 0 additions & 88 deletions src/node/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,94 +24,6 @@ export function createRequire(
return moduleUtil.createRequire(normalizedPath);
}

// Indicates only that the given specifier is known to be a
// Node.js built-in module specifier with or with the the
// 'node:' prefix. A true return value does not guarantee that
// the module is actually implemented in the runtime.
export function isBuiltin(specifier: string): boolean {
return moduleUtil.isBuiltin(specifier);
}

// Intentionally does not include modules with mandatory 'node:'
// prefix like `node:test`.
// See: See https://nodejs.org/docs/latest/api/modules.html#built-in-modules-with-mandatory-node-prefix
// TODO(later): This list duplicates the list that is in
// workerd/jsg/modules.c++. Later we should source these
// from the same place so we don't have to maintain two lists.
export const builtinModules = [
'_http_agent',
'_http_client',
'_http_common',
'_http_incoming',
'_http_outgoing',
'_http_server',
'_stream_duplex',
'_stream_passthrough',
'_stream_readable',
'_stream_transform',
'_stream_wrap',
'_stream_writable',
'_tls_common',
'_tls_wrap',
'assert',
'assert/strict',
'async_hooks',
'buffer',
'child_process',
'cluster',
'console',
'constants',
'crypto',
'dgram',
'diagnostics_channel',
'dns',
'dns/promises',
'domain',
'events',
'fs',
'fs/promises',
'http',
'http2',
'https',
'inspector',
'inspector/promises',
'module',
'net',
'os',
'path',
'path/posix',
'path/win32',
'perf_hooks',
'process',
'punycode',
'querystring',
'readline',
'readline/promises',
'repl',
'stream',
'stream/consumers',
'stream/promises',
'stream/web',
'string_decoder',
'sys',
'timers',
'timers/promises',
'tls',
'trace_events',
'tty',
'url',
'util',
'util/types',
'v8',
'vm',
'wasi',
'worker_threads',
'zlib',
];
Object.freeze(builtinModules);

export default {
createRequire,
isBuiltin,
builtinModules,
};
4 changes: 0 additions & 4 deletions src/workerd/api/node/module.c++
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@

namespace workerd::api::node {

bool ModuleUtil::isBuiltin(kj::String specifier) {
return jsg::checkNodeSpecifier(specifier) != kj::none;
}

jsg::JsValue ModuleUtil::createRequire(jsg::Lock& js, kj::String path) {
// Node.js requires that the specifier path is a File URL or an absolute
// file path string. To be compliant, we will convert whatever specifier
Expand Down
6 changes: 0 additions & 6 deletions src/workerd/api/node/module.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,8 @@ class ModuleUtil final: public jsg::Object {

jsg::JsValue createRequire(jsg::Lock& js, kj::String specifier);

// Returns true if the specifier is a known node.js built-in module specifier.
// Ignores whether or not the module actually exists (use process.getBuiltinModule()
// for that purpose).
bool isBuiltin(kj::String specifier);

JSG_RESOURCE_TYPE(ModuleUtil) {
JSG_METHOD(createRequire);
JSG_METHOD(isBuiltin);
}
};

Expand Down
23 changes: 1 addition & 22 deletions src/workerd/api/node/tests/module-create-require-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2017-2022 Cloudflare, Inc.
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
// https://opensource.org/licenses/Apache-2.0
import { createRequire, isBuiltin, builtinModules } from 'node:module';
import { createRequire } from 'node:module';
import { ok, strictEqual, throws } from 'node:assert';

export const doTheTest = {
Expand Down Expand Up @@ -52,24 +52,3 @@ export const doTheTest = {
createRequire(new URL('file:///'));
},
};

export const isBuiltinTest = {
test() {
ok(isBuiltin('fs'));
ok(isBuiltin('http'));
ok(isBuiltin('https'));
ok(isBuiltin('path'));
ok(isBuiltin('node:fs'));
ok(isBuiltin('node:http'));
ok(isBuiltin('node:https'));
ok(isBuiltin('node:path'));
ok(isBuiltin('node:test'));
ok(!isBuiltin('test'));
ok(!isBuiltin('worker'));
ok(!isBuiltin('worker/qux'));

builtinModules.forEach((module) => {
ok(isBuiltin(module));
});
},
};
Loading