Breaking changes v9.0.0 -> v9.1.0?

[jsjoeio]

Expected Behavior

ts-node executes start script and app starts on on port 8080 like it used to.

Actual Behavior

Strange behavior affecting between the child process and the parent process in our app. The child sends a message, the parent gets the process and sends a message back but the child never gets it.

Steps to reproduce the problem

1. git clone https://github.com/cdr/code-server.git && git checkout jsjoeio/ts-node-repro
2. yarn && yarn add -D ts-node@9.1.0
3. yarn watch
4. See output in console (app never starts as it should on port 8080)

Minimal reproduction

^See reproduction steps above. If that doesn't suffice or overcomplicates things, feel free to close this issue.

Specifications

• ts-node v9.1.0
• node v12.16.1
• compiler v4.2.3
• tsconfig.json, if you're using one:

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "moduleResolution": "node",
    "strict": true,
    "noImplicitReturns": true,
    "noUnusedLocals": true,
    "forceConsistentCasingInFileNames": true,
    "outDir": "./out",
    "declaration": true,
    "experimentalDecorators": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "tsBuildInfoFile": "./.cache/tsbuildinfo",
    "incremental": true,
    "typeRoots": ["./node_modules/@types", "./typings", "./test/node_modules/@types"],
    "downlevelIteration": true
  },
  "include": ["./src/**/*.ts"],
  "exclude": ["/test", "/lib", "/ci", "/doc"]
}

• Operating system and version: macOS Big Sur 11.2.2 (20D80)
• If Windows, are you using WSL or WSL2?: N/A

Additional Context

The ts-node upgrade came through dependabot in coder/code-server#2919. Our tests were passing so we thought it was fine to merge. (And now we have a note to write a test for testing our dev workflow, including use of ts-node to prevent this in the future).

After hours of debugging yesterday, my co-maintainer and I realized something in 9.1.0 (tested both 9.1.0 and 9.1.1) started causing issues. We looked at the release notes but didn't see anything unusual.

I figured it could be helpful to report this just in case.

[cspotcode]

Is there a list of binaries which must be globally installed to reproduce this? I'm seeing errors about jq, gulp, and npm-run-all.

[cspotcode]

These are the errors preventing the reproduction from working as expected. (I assume)
https://github.com/TypeStrong/ts-node-repros/runs/2197059246

Is the reproduction incomplete? Are there additional steps which need to be run?

[jsjoeio]

I'm seeing errors about jq, gulp, and npm-run-all

Apologies! Those are probably required for VS Code.

I spoke to @code-asher and he was able to provide a minimal reproduction setup. Here's the repo:
https://github.com/code-asher/ts-node-repro

[jsjoeio]

Okay so we couldn't reproduce in the minimal reproduction.

One observation: we're both testing using Node v12, but the repro GH action suite uses LTS. If you run Node v14, the issue isn't reproducible (locally).

And in 9.1.0, the tests for 12.16 were removed: #1148

So I guess that means this is a bug in Node and not ts-node?

[code-asher]

fwiw, I reproduced in GH actions with v12: TypeStrong/ts-node-repros#8

[cspotcode]

Thanks, I'm seeing the same. I see it on node 12.21.0 but not node 15.12.0. I suspect this has something to do with the way node's child_process module sets up the IPC channel.

[cspotcode]

I found something interesting. Switching the stdio configuration for the child_process fixes the problem.

    stdio: ['inherit', 'pipe', 'pipe', "ipc"], // <-- this works on node 12
    // stdio: ["ipc"], <--- this exhibits the failure we are seeing on node 12

Do you know, are we allowed to use file descriptor 0 for an IPC channel? If so, how does the child process learn that file descriptor 0 is an IPC channel?

I am also double-checking to be sure that ts-node is properly setting up process.execPath, process.execArgv, process.argv0, and process.argv.

[jsjoeio]

That is a fantastic question...This is waay out of my wheelhouse. I have no idea.

[code-asher]

Nice find! I'm not entirely sure why we're using fd 0 for IPC. 🤔 It was probably due to some kind of misunderstanding.

We'll switch to doing it the correct way. I have no idea if IPC is "supposed" to work on fd 0 so I don't know if this is still technically a bug, but this resolves our issue at the very least! Thank you for figuring this out!

[jsjoeio]

Feel free to close this if you'd like! You ended up helping us find the underlying bug in our own codebase and we really appreciate that!

[cspotcode]

Excellent, glad to hear that. I'm going to keep this open a bit longer because it might still be a bug on our end, or at least something we can avoid, if I can figure out what to tweak.

…

On Mon, Apr 5, 2021 at 3:15 PM Joe Previte ***@***.***> wrote: Feel free to close this if you'd like! You ended up helping us find the underlying bug in our own codebase and we really appreciate that! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1283 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC35OCVLIGXX5QNTFOHVOLTHID6PANCNFSM4ZYK3WSQ> .

[cspotcode]

I came back to this and found the culprit.

Doing const _thisIsNeverUsed = process.stdin; triggers this bug in vanilla node without ts-node. ts-node >= 9.1.0 eagerly saves a reference to process.stdin, which is why we were seeing the bug triggered by ts-node.

I guess that node 12.16.1 had some initialize-on-demand behavior that was triggered by a getter on process.stdin. Seems rare enough not to warrant a workaround.

However, if we did need to implement a workaround, the solution is to avoid calling createRepl in bin.ts unless the REPL will be used. createRepl() internally saves a reference to process.stdin. Avoiding that call avoids triggering this bug.

It is worth repeating for anyone reading this in the future: this bug does not happen on newer versions of node.