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

module: allow running .ts in node_modules if private #55385

Closed
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
3 changes: 2 additions & 1 deletion doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -3041,7 +3041,8 @@ import 'package-name'; // supported
added: v22.6.0
-->

Type stripping is not supported for files descendent of a `node_modules` directory.
Type stripping is not supported for files descendent of a `node_modules` directory,
where package.json is missing or does not contain the property `"private": true`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
where package.json is missing or does not contain the property `"private": true`.
where package.json is missing or does not contain the property `"private": true`.

Is there any chance it can be missing?


<a id="ERR_UNSUPPORTED_RESOLVE_REQUEST"></a>

Expand Down
2 changes: 1 addition & 1 deletion doc/api/typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ are enabled by default.

To discourage package authors from publishing packages written in TypeScript,
Node.js will by default refuse to handle TypeScript files inside folders under
a `node_modules` path.
a `node_modules` path, unless the package.json contains the property `"private": true`.

[CommonJS]: modules.md
[ES Modules]: esm.md
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1839,7 +1839,8 @@ E('ERR_UNSUPPORTED_ESM_URL_SCHEME', (url, supported) => {
return msg;
}, Error);
E('ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING',
'Stripping types is currently unsupported for files under node_modules, for "%s"',
'Stripping types is currently unsupported for files under node_modules that ' +
'do not contain a package.json with the property private: true, for "%s"',
Error);
E('ERR_UNSUPPORTED_RESOLVE_REQUEST',
'Failed to resolve module specifier "%s" from "%s": Invalid relative URL or base scheme is not hierarchical.',
Expand Down
16 changes: 3 additions & 13 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ const { pathToFileURL, fileURLToPath, isURL } = require('internal/url');
const {
pendingDeprecate,
emitExperimentalWarning,
isUnderNodeModules,
kEmptyObject,
setOwnProperty,
getLazy,
Expand Down Expand Up @@ -170,7 +169,6 @@ const {
ERR_REQUIRE_CYCLE_MODULE,
ERR_REQUIRE_ESM,
ERR_UNKNOWN_BUILTIN_MODULE,
ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING,
},
setArrowMessage,
} = require('internal/errors');
Expand Down Expand Up @@ -1370,9 +1368,6 @@ let hasPausedEntry = false;
function loadESMFromCJS(mod, filename) {
let source = getMaybeCachedSource(mod, filename);
if (getOptionValue('--experimental-strip-types') && path.extname(filename) === '.mts') {
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}
source = stripTypeScriptTypes(source, filename);
}
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
Expand Down Expand Up @@ -1594,9 +1589,6 @@ function getMaybeCachedSource(mod, filename) {
}

function loadCTS(module, filename) {
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}
const source = getMaybeCachedSource(module, filename);
const code = stripTypeScriptTypes(source, filename);
module._compile(code, filename, 'commonjs');
Expand All @@ -1608,14 +1600,12 @@ function loadCTS(module, filename) {
* @param {string} filename The file path of the module
*/
function loadTS(module, filename) {
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}
// If already analyzed the source, then it will be cached.
const source = getMaybeCachedSource(module, filename);
const content = stripTypeScriptTypes(source, filename);
let format;
const pkg = packageJsonReader.getNearestParentPackageJSON(filename);
const isPrivatePackageJSON = pkg?.data.private;
const content = stripTypeScriptTypes(source, filename, isPrivatePackageJSON);
// Function require shouldn't be used in ES modules.
if (pkg?.data.type === 'module') {
if (getOptionValue('--experimental-require-module')) {
Expand All @@ -1633,7 +1623,7 @@ function loadTS(module, filename) {
if (Module._cache[parentPath]) {
let parentSource;
try {
parentSource = stripTypeScriptTypes(fs.readFileSync(parentPath, 'utf8'), parentPath);
parentSource = stripTypeScriptTypes(fs.readFileSync(parentPath, 'utf8'), parentPath, isPrivatePackageJSON);
} catch {
// Continue regardless of error.
}
Expand Down
8 changes: 0 additions & 8 deletions lib/internal/modules/esm/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const {
RegExpPrototypeExec,
} = primordials;
const {
isUnderNodeModules,
kEmptyObject,
} = require('internal/util');

Expand All @@ -23,7 +22,6 @@ const {
ERR_INVALID_URL,
ERR_UNKNOWN_MODULE_FORMAT,
ERR_UNSUPPORTED_ESM_URL_SCHEME,
ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING,
} = require('internal/errors').codes;

const {
Expand Down Expand Up @@ -131,12 +129,6 @@ async function defaultLoad(url, context = kEmptyObject) {

validateAttributes(url, format, importAttributes);

if (getOptionValue('--experimental-strip-types') &&
(format === 'module-typescript' || format === 'commonjs-typescript') &&
isUnderNodeModules(url)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(url);
}

return {
__proto__: null,
format,
Expand Down
27 changes: 25 additions & 2 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_RETURN_PROPERTY_VALUE,
ERR_INVALID_TYPESCRIPT_SYNTAX,
ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING,
} = require('internal/errors').codes;
const { BuiltinModule } = require('internal/bootstrap/realm');

Expand All @@ -28,7 +29,7 @@ const assert = require('internal/assert');

const { Buffer } = require('buffer');
const { getOptionValue } = require('internal/options');
const { assertTypeScript, setOwnProperty, getLazy } = require('internal/util');
const { assertTypeScript, setOwnProperty, getLazy, isUnderNodeModules } = require('internal/util');
const { inspect } = require('internal/util/inspect');

const lazyTmpdir = getLazy(() => require('os').tmpdir());
Expand Down Expand Up @@ -347,6 +348,21 @@ function parseTypeScript(source, options) {
}
}

/**
*
* @param {string} filename
* @returns {boolean} Returns true if the nearest package.json is private.
*/
function isPrivateNodeModule(filename) {
const packageJsonReader = require('internal/modules/package_json_reader');
const resolved = StringPrototypeStartsWith(filename, 'file://') ? filename : pathToFileURL(filename).href;
const packageConfig = packageJsonReader.getPackageScopeConfig(resolved);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we cache already analized paths?
Like everything under node_modules/foo once we have already read read node_modules/foo/package.json

if (!packageConfig?.exists) {
return false;
}
return packageConfig.isPrivate === true;
}

/**
* @typedef {object} TransformOutput
* @property {string} code The compiled code.
Expand All @@ -355,9 +371,16 @@ function parseTypeScript(source, options) {
* Performs type-stripping to TypeScript source code.
* @param {string} source TypeScript code to parse.
* @param {string} filename The filename of the source code.
* @param {string} isPackageJsonPrivate Whether the nearest package.json is private.
* @returns {TransformOutput} The stripped TypeScript code.
*/
function stripTypeScriptTypes(source, filename) {
function stripTypeScriptTypes(source, filename, isPackageJsonPrivate = false) {
// If the package.json is private, we can
// allow the stripping of types because it is
// not possible to publish it on npm.
if (isUnderNodeModules(filename) && isPackageJsonPrivate !== true && !isPrivateNodeModule(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}
assert(typeof source === 'string');
const options = {
__proto__: null,
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/modules/package_json_reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ function deserializePackageJSON(path, contents) {
3: plainImports,
4: plainExports,
5: optionalFilePath,
6: isPrivate,
} = contents;

// This is required to be used in getPackageScopeConfig.
Expand Down Expand Up @@ -64,6 +65,7 @@ function deserializePackageJSON(path, contents) {
ObjectDefineProperty(this, 'exports', { __proto__: null, value });
return this.exports;
},
isPrivate,
};
}

Expand Down
10 changes: 8 additions & 2 deletions src/node_modules.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace node {
namespace modules {

using v8::Array;
using v8::Boolean;
using v8::Context;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
Expand Down Expand Up @@ -74,15 +75,16 @@ Local<Array> BindingData::PackageConfig::Serialize(Realm* realm) const {
isolate, input.data(), NewStringType::kNormal, input.size())
.ToLocalChecked();
};
Local<Value> values[6] = {
Local<Value> values[7] = {
name.has_value() ? ToString(*name) : Undefined(isolate),
main.has_value() ? ToString(*main) : Undefined(isolate),
ToString(type),
imports.has_value() ? ToString(*imports) : Undefined(isolate),
exports.has_value() ? ToString(*exports) : Undefined(isolate),
ToString(file_path),
Boolean::New(isolate, is_private),
};
return Array::New(isolate, values, 6);
return Array::New(isolate, values, 7);
}

const BindingData::PackageConfig* BindingData::GetPackageJSON(
Expand Down Expand Up @@ -225,6 +227,10 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
default:
break;
}
} else if (key == "private") {
if (value.get_bool().get(package_config.is_private)) {
return throw_invalid_package_config();
}
}
}
// package_config could be quite large, so we should move it instead of
Expand Down
1 change: 1 addition & 0 deletions src/node_modules.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class BindingData : public SnapshotableObject {
std::optional<std::string> imports;
std::optional<std::string> scripts;
std::string raw_json;
bool is_private;

v8::Local<v8::Array> Serialize(Realm* realm) const;
};
Expand Down
12 changes: 12 additions & 0 deletions test/es-module/test-typescript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -414,3 +414,15 @@ test('expect error when executing a TypeScript file with generics', async () =>
strictEqual(result.stdout, '');
strictEqual(result.code, 1);
});

test('execute TypeScript file from node_modules with private: true', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-strip-types',
'--no-warnings',
fixtures.path('typescript/ts/test-import-private-ts-node-module.ts'),
]);

strictEqual(result.stderr, '');
match(result.stdout, /Hello, TypeScript!/);
strictEqual(result.code, 0);
});
1 change: 1 addition & 0 deletions test/fixtures/typescript/ts/node_modules/bar/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions test/fixtures/typescript/ts/node_modules/private/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { foo } from 'private';

interface Bar { };

console.log(foo);
4 changes: 3 additions & 1 deletion typings/internalBinding/modules.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ export type PackageConfig = {
main?: any
type: PackageType
exports?: string | string[] | Record<string, unknown>
imports?: string | string[] | Record<string, unknown>
imports?: string | string[] | Record<string, unknown>,
isPrivate?: boolean
}
export type SerializedPackageConfig = [
PackageConfig['name'],
Expand All @@ -15,6 +16,7 @@ export type SerializedPackageConfig = [
string | undefined, // exports
string | undefined, // imports
string | undefined, // raw json available for experimental policy
boolean | undefined // isPrivate
]

export interface ModulesBinding {
Expand Down
Loading