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
Allow specifying "require" option via tsconfig #925
Changes from 27 commits
7e301ef
73a76f3
91ca892
e11be56
5b57626
d14a028
973a78c
7dc0009
3fa74a5
90e059d
7050acc
b9b2f67
91f5e13
a209bdf
700a90f
80ea06c
e7690b9
750dc00
8a5573b
362db15
567582e
3eb3deb
bbca6a6
1c500d5
a9ff8c9
bd53d10
544d1bf
18566b4
6fb4b8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Extracted from https://github.com/nodejs/node/blob/ec2ffd6b9d255e19818b6949d2f7dc7ac70faee9/lib/internal/modules/cjs/loader.js | ||
// then modified to suit our needs | ||
|
||
const path = require('path'); | ||
const Module = require('module'); | ||
|
||
exports.createRequireFromPath = createRequireFromPath; | ||
|
||
function createRequireFromPath(filename) { | ||
// Allow a directory to be passed as the filename | ||
const trailingSlash = | ||
filename.endsWith('/') || (isWindows && filename.endsWith('\\')); | ||
|
||
const proxyPath = trailingSlash ? | ||
path.join(filename, 'noop.js') : | ||
filename; | ||
|
||
const m = new Module(proxyPath); | ||
m.filename = proxyPath; | ||
|
||
m.paths = Module._nodeModulePaths(m.path); | ||
return makeRequireFunction(m, proxyPath); | ||
} | ||
|
||
// This trick is much smaller than copy-pasting from https://github.com/nodejs/node/blob/ec2ffd6b9d255e19818b6949d2f7dc7ac70faee9/lib/internal/modules/cjs/helpers.js#L32-L101 | ||
function makeRequireFunction(module, filename) { | ||
module._compile('module.exports = require;', filename) | ||
return mod.exports | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import * as ynModule from 'yn' | |
import { BaseError } from 'make-error' | ||
import * as util from 'util' | ||
import * as _ts from 'typescript' | ||
import * as Module from 'module' | ||
|
||
/** | ||
* Does this version of node obey the package.json "type" field | ||
|
@@ -218,6 +219,19 @@ export interface RegisterOptions extends CreateOptions { | |
preferTsExts?: boolean | ||
} | ||
|
||
/** | ||
* Options only available via tsconfig, not register() nor create() APIs. | ||
*/ | ||
export interface TsConfigExclusiveOptions { | ||
require?: Array<string> | ||
} | ||
|
||
/** | ||
* Options returned by us after we merge programmatic options with those | ||
* from tsconfig.json | ||
*/ | ||
export interface ParsedOptions extends RegisterOptions, TsConfigExclusiveOptions {} | ||
|
||
/** | ||
* Must be an interface to support `typescript-json-schema`. | ||
*/ | ||
|
@@ -228,7 +242,7 @@ export interface TsConfigOptions extends Omit<RegisterOptions, | |
| 'skipProject' | ||
| 'project' | ||
| 'dir' | ||
> { } | ||
>, TsConfigExclusiveOptions {} | ||
|
||
/** | ||
* Like `Object.assign`, but ignores `undefined` properties. | ||
|
@@ -333,7 +347,7 @@ export class TSError extends BaseError { | |
export interface Register { | ||
ts: TSCommon | ||
config: _ts.ParsedCommandLine | ||
options: RegisterOptions | ||
options: ParsedOptions | ||
enabled (enabled?: boolean): boolean | ||
ignored (fileName: string): boolean | ||
compile (code: string, fileName: string, lineOffset?: number): string | ||
|
@@ -408,7 +422,7 @@ export function create (rawOptions: CreateOptions = {}): Register { | |
|
||
// Read config file and merge new options between env and CLI options. | ||
const { config, options: tsconfigOptions } = readConfig(cwd, ts, rawOptions) | ||
const options = assign<CreateOptions>({}, DEFAULTS, tsconfigOptions || {}, rawOptions) | ||
const options = assign<ParsedOptions>({}, DEFAULTS, tsconfigOptions || {}, rawOptions) | ||
|
||
// If `compiler` option changed based on tsconfig, re-load the compiler. | ||
if (options.compiler !== compilerName) { | ||
|
@@ -991,6 +1005,14 @@ function readConfig ( | |
useCaseSensitiveFileNames: ts.sys.useCaseSensitiveFileNames | ||
}, basePath, undefined, configFileName)) | ||
|
||
if (tsconfigOptions.require) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on doing the require for these options within There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I like that. The CLI uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I recall I tried to change it one time. Feel free to keep the CLI logic as it is and build the internal one separately for now so there's no regression. Here's the implementation in node.js: https://github.com/nodejs/node/blob/ec2ffd6b9d255e19818b6949d2f7dc7ac70faee9/lib/internal/modules/cjs/loader.js#L1400-L1417. Here's where I had changed it at one point, but I can't seem to find the issue. I should have linked things better in the past 🤕 2e99c50#diff-ee45801c137c3b9f9d8ec7ece340a514R197. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main thing it's trying to ensure is that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I don't think there's anything wrong with calling Should it happen in |
||
// Relative paths are relative to the tsconfig's parent directory, not the `dir` option | ||
const tsconfigRelativeRequire = createRequire(configFileName!) | ||
tsconfigOptions.require = tsconfigOptions.require.map((path: string) => { | ||
return tsconfigRelativeRequire.resolve(path) | ||
}) | ||
} | ||
|
||
return { config: fixedConfig, options: tsconfigOptions } | ||
} | ||
|
||
|
@@ -1051,3 +1073,12 @@ function getTokenAtPosition (ts: typeof _ts, sourceFile: _ts.SourceFile, positio | |
return current | ||
} | ||
} | ||
|
||
let nodeCreateRequire: (path: string) => NodeRequire | ||
function createRequire (filename: string) { | ||
if (!nodeCreateRequire) { | ||
// tslint:disable-next-line | ||
nodeCreateRequire = Module.createRequire || Module.createRequireFromPath || require('../dist-raw/node-createrequire').createRequireFromPath | ||
} | ||
return nodeCreateRequire(filename) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
process.required = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we merge these lists together? Seems odd to have the CLI entirely overwrite the options when specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense. I can't think of a good use-case where you'd want the
--require
flag to suppress whatever was specified in the tsconfig.