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

Allow specifying "require" option via tsconfig #925

Merged
merged 29 commits into from Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7e301ef
WIP
cspotcode Nov 29, 2019
73a76f3
WIP
cspotcode Dec 4, 2019
91ca892
WIP tests
cspotcode Dec 5, 2019
e11be56
WIP
cspotcode Dec 5, 2019
5b57626
fix
cspotcode Dec 5, 2019
d14a028
fix
cspotcode Dec 5, 2019
973a78c
Fix
cspotcode Dec 5, 2019
7dc0009
Addressing PR feedback. Not done yet; just pushing what I have so far
cspotcode Dec 5, 2019
3fa74a5
Pushing fixes to view the gh diff
cspotcode Dec 14, 2019
90e059d
Pushing fixes to view the gh diff
cspotcode Dec 14, 2019
7050acc
Fixes
cspotcode Dec 14, 2019
b9b2f67
comments
cspotcode Dec 14, 2019
91f5e13
Cleanup
cspotcode Dec 14, 2019
a209bdf
Finalize tsconfig PR
cspotcode Dec 14, 2019
700a90f
Remove ability to specify "requires" in tsconfig
cspotcode Dec 14, 2019
80ea06c
Fix linter failures
cspotcode Dec 14, 2019
e7690b9
Revert "Remove ability to specify "requires" in tsconfig"
cspotcode Dec 14, 2019
750dc00
Merge remote-tracking branch 'origin/master' into ts-node-config-requ…
cspotcode Feb 11, 2020
8a5573b
remove duplication from merge
cspotcode Feb 11, 2020
362db15
comments
cspotcode Feb 11, 2020
567582e
fix
cspotcode Feb 11, 2020
3eb3deb
Merge remote-tracking branch 'origin/master' into HEAD
cspotcode Jul 27, 2020
bbca6a6
Fix
cspotcode Jul 28, 2020
1c500d5
fix
cspotcode Jul 28, 2020
a9ff8c9
fix
cspotcode Jul 28, 2020
bd53d10
fix
cspotcode Jul 28, 2020
544d1bf
fix
cspotcode Jul 28, 2020
18566b4
Merge tsconfig and programmatic / CLI `require` arrays; load them in …
cspotcode Jul 29, 2020
6fb4b8f
tests
cspotcode Jul 29, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 29 additions & 0 deletions dist-raw/node-createrequire.js
@@ -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
}
6 changes: 2 additions & 4 deletions src/bin.ts
Expand Up @@ -90,7 +90,7 @@ export function main (argv: string[]) {
'--help': help = false,
'--script-mode': scriptMode = false,
'--version': version = 0,
'--require': requires = [],
'--require': argsRequire = [],
'--eval': code = undefined,
'--print': print = false,
'--interactive': interactive = false,
Expand Down Expand Up @@ -176,6 +176,7 @@ export function main (argv: string[]) {
compiler,
ignoreDiagnostics,
compilerOptions,
require: argsRequire,
readFile: code !== undefined
? (path: string) => {
if (path === state.path) return state.input
Expand Down Expand Up @@ -212,9 +213,6 @@ export function main (argv: string[]) {
module.filename = state.path
module.paths = (Module as any)._nodeModulePaths(cwd)

// Require specified modules before start-up.
;(Module as any)._preloadModules(requires)

// Prepend `ts-node` arguments to CLI for child processes.
process.execArgv.unshift(__filename, ...process.argv.slice(2, process.argv.length - args._.length))
process.argv = [process.argv[1]].concat(scriptPath || []).concat(args._.slice(1))
Expand Down
16 changes: 11 additions & 5 deletions src/index.spec.ts
Expand Up @@ -471,7 +471,7 @@ describe('ts-node', function () {
const BIN_EXEC = `"${BIN_PATH}" --project tests/tsconfig-options/tsconfig.json`

it('should override compiler options from env', function (done) {
exec(`${BIN_EXEC} tests/tsconfig-options/log-options.js`, {
exec(`${BIN_EXEC} tests/tsconfig-options/log-options1.js`, {
env: {
...process.env,
TS_NODE_COMPILER_OPTIONS: '{"typeRoots": ["env-typeroots"]}'
Expand All @@ -485,33 +485,38 @@ describe('ts-node', function () {
})

it('should use options from `tsconfig.json`', function (done) {
exec(`${BIN_EXEC} tests/tsconfig-options/log-options.js`, function (err, stdout) {
exec(`${BIN_EXEC} tests/tsconfig-options/log-options1.js`, function (err, stdout) {
expect(err).to.equal(null)
const { options, config } = JSON.parse(stdout)
expect(config.options.typeRoots).to.deep.equal([join(__dirname, '../tests/tsconfig-options/tsconfig-typeroots').replace(/\\/g, '/')])
expect(config.options.types).to.deep.equal(['tsconfig-tsnode-types'])
expect(options.pretty).to.equal(undefined)
expect(options.skipIgnore).to.equal(false)
expect(options.transpileOnly).to.equal(true)
expect(options.require).to.deep.equal([join(__dirname, '../tests/tsconfig-options/required1.js')])
return done()
})
})

it('should have flags override `tsconfig.json`', function (done) {
exec(`${BIN_EXEC} --skip-ignore --compiler-options "{\\"types\\":[\\"flags-types\\"]}" tests/tsconfig-options/log-options.js`, function (err, stdout) {
it('should have flags override / merge with `tsconfig.json`', function (done) {
exec(`${BIN_EXEC} --skip-ignore --compiler-options "{\\"types\\":[\\"flags-types\\"]}" --require ./tests/tsconfig-options/required2.js tests/tsconfig-options/log-options2.js`, function (err, stdout) {
expect(err).to.equal(null)
const { options, config } = JSON.parse(stdout)
expect(config.options.typeRoots).to.deep.equal([join(__dirname, '../tests/tsconfig-options/tsconfig-typeroots').replace(/\\/g, '/')])
expect(config.options.types).to.deep.equal(['flags-types'])
expect(options.pretty).to.equal(undefined)
expect(options.skipIgnore).to.equal(true)
expect(options.transpileOnly).to.equal(true)
expect(options.require).to.deep.equal([
join(__dirname, '../tests/tsconfig-options/required1.js'),
'./tests/tsconfig-options/required2.js'
])
return done()
})
})

it('should have `tsconfig.json` override environment', function (done) {
exec(`${BIN_EXEC} tests/tsconfig-options/log-options.js`, {
exec(`${BIN_EXEC} tests/tsconfig-options/log-options1.js`, {
env: {
...process.env,
TS_NODE_PRETTY: 'true',
Expand All @@ -525,6 +530,7 @@ describe('ts-node', function () {
expect(options.pretty).to.equal(true)
expect(options.skipIgnore).to.equal(false)
expect(options.transpileOnly).to.equal(true)
expect(options.require).to.deep.equal([join(__dirname, '../tests/tsconfig-options/required1.js')])
return done()
})
})
Expand Down
38 changes: 36 additions & 2 deletions src/index.ts
Expand Up @@ -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
Expand Down Expand Up @@ -195,6 +196,15 @@ export interface CreateOptions {
* Ignore TypeScript warnings by diagnostic code.
*/
ignoreDiagnostics?: Array<number | string>
/**
* Modules to require, like node's `--require` flag.
*
* If specified in tsconfig.json, the modules will be resolved relative to the tsconfig.json file.
*
* If specified programmatically, each input string should be pre-resolved to an absolute path for
* best results.
*/
require?: Array<string>
readFile?: (path: string) => string | undefined
fileExists?: (path: string) => boolean
transformers?: _ts.CustomTransformers | ((p: _ts.Program) => _ts.CustomTransformers)
Expand Down Expand Up @@ -228,7 +238,7 @@ export interface TsConfigOptions extends Omit<RegisterOptions,
| 'skipProject'
| 'project'
| 'dir'
> { }
> {}

/**
* Like `Object.assign`, but ignores `undefined` properties.
Expand Down Expand Up @@ -382,6 +392,9 @@ export function register (opts: RegisterOptions = {}): Register {
// Register the extensions.
registerExtensions(service.options.preferTsExts, extensions, service, originalJsHandler)

// Require specified modules before start-up.
;(Module as any)._preloadModules(service.options.require)

return service
}

Expand All @@ -408,7 +421,11 @@ 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<RegisterOptions>({}, DEFAULTS, tsconfigOptions || {}, rawOptions)
options.require = [
...tsconfigOptions.require || [],
...rawOptions.require || []
]

// If `compiler` option changed based on tsconfig, re-load the compiler.
if (options.compiler !== compilerName) {
Expand Down Expand Up @@ -991,6 +1008,14 @@ function readConfig (
useCaseSensitiveFileNames: ts.sys.useCaseSensitiveFileNames
}, basePath, undefined, configFileName))

if (tsconfigOptions.require) {
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts on doing the require for these options within index.ts so configuration flows in a single direction? It might make it easier instead of needing to resolve and pass it back to the CLI which requires. Also, this would ensure we cover the programmatic usage of register() and via ts-node/register.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I like that.

The CLI uses Module._preloadModules; do you know if that function is doing anything special that will break if we switch to a normal require() call? There's nothing wrong with _preloadModules but I don't really understand what it does other than require() stuff.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

The main thing it's trying to ensure is that the require() is occurring from the place where you run the script, not where ts-node lives.

Copy link
Collaborator Author

@cspotcode cspotcode Jul 29, 2020

Choose a reason for hiding this comment

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

Ok, I don't think there's anything wrong with calling _preloadModules from within register(), so I'll keep doing that.

Should it happen in register() or in create()? I feel like register() makes more sense because that's where we "install" the environment. But source map support is installed in create().

// Modules are found relative to the tsconfig file, not the `dir` option
const tsconfigRelativeRequire = createRequire(configFileName!)
tsconfigOptions.require = tsconfigOptions.require.map((path: string) => {
return tsconfigRelativeRequire.resolve(path)
})
}

return { config: fixedConfig, options: tsconfigOptions }
}

Expand Down Expand Up @@ -1051,3 +1076,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)
}
@@ -1,4 +1,5 @@
const assert = require('assert')
assert(process.required1)
const register = process[Symbol.for('ts-node.register.instance')]
console.log(JSON.stringify({
options: register.options,
Expand Down
3 changes: 3 additions & 0 deletions tests/tsconfig-options/log-options2.js
@@ -0,0 +1,3 @@
const assert = require('assert')
require('./log-options1')
assert(process.required2)
1 change: 1 addition & 0 deletions tests/tsconfig-options/required1.js
@@ -0,0 +1 @@
process.required1 = true
2 changes: 2 additions & 0 deletions tests/tsconfig-options/required2.js
@@ -0,0 +1,2 @@
require('assert')(process.required1)
process.required2 = true
1 change: 1 addition & 0 deletions tests/tsconfig-options/tsconfig.json
Expand Up @@ -5,6 +5,7 @@
"types": ["tsconfig-tsnode-types"]
},
"transpileOnly": true,
"require": ["./required1"],
"skipIgnore": false
},
"compilerOptions": {
Expand Down