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

REPL, -e, and piped stdin globals are wrong: ,missing exports, module, etc. #1282

Closed
cspotcode opened this issue Mar 22, 2021 · 6 comments · Fixed by #1333
Closed

REPL, -e, and piped stdin globals are wrong: ,missing exports, module, etc. #1282

cspotcode opened this issue Mar 22, 2021 · 6 comments · Fixed by #1333
Milestone

Comments

@cspotcode
Copy link
Collaborator

Expected Behavior

ts-node --interactive -e "<expression>" should behave the same as node --interactive -e "<expression>"

-e should be evaluated with exports, etc.

Actual Behavior

exports is not defined during the evaluation of a -e expression.

Steps to reproduce the problem / Minimal reproduction

ts-node --interactive -e "console.log(exports, __filename)"
node --interactive -e "console.log(exports, __filename)"

Notes

Looks like a blind spot in our REPL logic in bin.ts We should check that all the module-specific variables, exports, __filename, etc match nodejs.

@cspotcode
Copy link
Collaborator Author

Seems this bit of code, which only runs when you call start(), should actually be run earlier in some of our -e codepaths.
https://github.com/TypeStrong/ts-node/blob/main/src/repl.ts#L263-L268

@cspotcode cspotcode added this to the next milestone May 15, 2021
@cspotcode cspotcode modified the milestones: next, 10.0.0 May 23, 2021
@cspotcode
Copy link
Collaborator Author

Did some more research here.

node --interactive REPL uses the global object as the REPL's scope. We opt-in to the same via the API.

node -e '< expression >' gets access to a bunch of variables added to the global object.
So does REPL code.

I did a test logging the set of fields on global which are added by calling new REPLServer()

[
  'module',      'require',             'assert',
  'async_hooks', 'buffer',              'child_process',
  'cluster',     'constants',           'crypto',
  'dgram',       'diagnostics_channel', 'dns',
  'domain',      'events',              'fs',
  'http',        'http2',               'https',
  'inspector',   'net',                 'os',
  'path',        'perf_hooks',          'punycode',
  'querystring', 'readline',            'repl',
  'stream',      'string_decoder',      'sys',
  'timers',      'tls',                 'trace_events',
  'tty',         'url',                 'util',
  'v8',          'vm',                  'worker_threads',
  'zlib',        '_',                   '_error'
]

The modules like fs are getters, presumably for performance.

global.__filename and global.__dirname are set for -e or -p, but only stick around on the global object if you do node -p "" -i

__filename is [eval] for -e, [stdin] for piped input.

stdin and -e global.module, .exports, .require, .__filename, .__dirname set.

new REPLServer() only sets .module, .require, but includes _ and _error.

console is not overwritten. I thought it might have been to redirect console.log() to the stdin and stdout passed to the REPL, but no.

Looks like we need to do these things:

  • call new REPLServer() before any sort of -e, -p, -i, or piped stdin
  • set __filename and __dirname only in certain codepaths
  • choose between [eval] and [stdin] for __filename
  • Continue to always set exports because TS emitted code requires it
  • update REPL API to let you opt-out of using the global object as context
  • ensure REPL API exposes .context
  • Consider adding call to REPL API to create REPLServer without starting it

@cspotcode
Copy link
Collaborator Author

cspotcode commented May 23, 2021

Also must be sure module.paths matches in all cases. Appears to differ between -p and repl.

❯ node -p '[module.id, module.paths]' -i
Welcome to Node.js v16.1.0.
Type ".help" for more information.
> [
  '[eval]',
  [
    '/d/Personal-dev/@TypeStrong/ts-node/node_modules',
    '/d/Personal-dev/@TypeStrong/node_modules',
    '/d/Personal-dev/node_modules',
    '/d/node_modules',
    '/node_modules'
  ]
]
> [[module.id, module.paths]
... [module.id, module.paths]
...
> [module.id, module.paths]
[
  '<repl>',
  [
    '/d/Personal-dev/@TypeStrong/ts-node/repl/node_modules',
    '/d/Personal-dev/@TypeStrong/ts-node/node_modules',
    '/d/Personal-dev/@TypeStrong/node_modules',
    '/d/Personal-dev/node_modules',
    '/d/node_modules',
    '/node_modules',
    '/home/cspotcode/.node_modules',
    '/home/cspotcode/.node_libraries',
    '/home/cspotcode/n/lib/node'
  ]
]
>

@cspotcode
Copy link
Collaborator Author

cspotcode commented May 24, 2021

Discrepancy between:

node -p 'process.argv' ./subdir/index.ts
ts-node -p -e 'process.argv' ./subdir/index.ts

node treats the positional as an argv string, does NOT attempt to execute it.

ts-node breaks:

  • attempts to resolve it to a file; fails if doesn't exist
  • uses the path to set the location of internal EvalState
  • TS compiler complains if it points to a file that should not be compiled

@cspotcode
Copy link
Collaborator Author

cspotcode commented May 24, 2021

Our repl does process.exit(). We should make the REPL API raise an error somehow, and call process.exit() in response to that error in our bin.ts.

@cspotcode cspotcode changed the title ts-node --interactive -e evaluates -e expression without exports, module, etc. REPL, -e, and piped stdin globals are wrong: ,missing exports, module, etc. May 27, 2021
@cspotcode
Copy link
Collaborator Author

Renamed to capture scope of this ticket.

@cspotcode cspotcode modified the milestones: next, 10.x.x May 27, 2021
cspotcode added a commit that referenced this issue Jun 8, 2021
…1333)

* add failing tests

* remove log statement from test

* add detailed tests for globals in <repl>, [stdin], and [eval]

* WIP fixing

* more WIP

* update tests

* WIP

* update packagelock

* fix tests

* Fix and tests

* fix test failure on windows

* fix programmatic test to cleanup potentially-polluted env prior

* modifying programmatic repl test to call out that `module` is unavailable before repl is started

* Update tests for Windows env

* Fix tests

* add retries around npm install in tests on windows

* lintfix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant