Skip to content

feat: add TypeScript to default execPath#1552

Merged
remy merged 1 commit into
remy:masterfrom
leonardodino:add-ts-exec-map
May 1, 2019
Merged

feat: add TypeScript to default execPath#1552
remy merged 1 commit into
remy:masterfrom
leonardodino:add-ts-exec-map

Conversation

@leonardodino

Copy link
Copy Markdown
Contributor

ts-node is the standard for running typescript node programs on development mode.

Adding this line will enable everyone with a tsconfig.json to have a full-refresh server watching experience. (:

`ts-node` is the standard for running typescript node programs on development mode.

Adding this line will enable everyone with a `tsconfig.json` to have a full-refresh server watching experience. (:
@stale

stale Bot commented Apr 24, 2019

Copy link
Copy Markdown

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up.
Thank you for contributing <3

@stale stale Bot added the stale no activity for 2 weeks label Apr 24, 2019
@remy

remy commented Apr 24, 2019 via email

Copy link
Copy Markdown
Owner

@stale stale Bot removed the stale no activity for 2 weeks label Apr 24, 2019
@leonardodino leonardodino changed the title Add TypeScript to default execPath feat: add TypeScript to default execPath Apr 24, 2019
@remy remy merged commit 64b474e into remy:master May 1, 2019
@leonardodino leonardodino deleted the add-ts-exec-map branch May 1, 2019 15:01
@teebot

teebot commented May 6, 2019

Copy link
Copy Markdown

It's nice but in my opinion it's inconsistent with node.

  • You still use node -r ts-node/register app.ts in production.
  • nodemon -r ts-node/register app.ts now triggers a TS compilation error

After my last npm upgrade it took me a while to figure out nodemon was the one to blame and that I had to remove my register argument.

@remy

remy commented May 6, 2019

Copy link
Copy Markdown
Owner

@teebot hmm, this seems rather important. Can you raise a separate issue (allowing others to find the problem).

It seems like we'll either need to be smart about adding ts support or removing it…

@teebot

teebot commented May 6, 2019

Copy link
Copy Markdown

Sure thing I created #1564
Due to limited time I'm not able to investigate and open a PR at the moment. Sorry about that..

@mefcorvi

mefcorvi commented May 6, 2019

Copy link
Copy Markdown

Well, this pull request explains why now I'm getting Unknown or unexpected option: --inspect error. The ts-node doesn't support that option and strictly speaking ts-node is not a drop-in replacement for node, in some cases you need to run node for ts files.

@leonardodino

leonardodino commented May 6, 2019

Copy link
Copy Markdown
Contributor Author

sorry guys, my bad.

You still use node -r ts-node/register app.ts in production.

I didn't knew people were running ts-node in production, I always compile first and use it only for development.

@teebot

teebot commented May 6, 2019 via email

Copy link
Copy Markdown

@leonardodino

leonardodino commented May 6, 2019

Copy link
Copy Markdown
Contributor Author

as of the last release, you can now replace:
nodemon -r ts-node/register app.ts => nodemon app.ts

and I think the equivalent for custom node options will be overriding the ts-node in package.json scripts with something along this lines:

"scripts": {
	"ts-node": "node -r ts-node/register",
	// ...other scripts
}

(haven't tested this last one, but I think it can work)

@mefcorvi

mefcorvi commented May 7, 2019

Copy link
Copy Markdown

and I think the equivalent for custom node options will be overriding the ts-node

I've solved my issue by overriding executable for "ts" files back to node in nodemon configurations :-) After all explicit is better than implicit. Not sure if I must close #1565 I think that it's worth to mention somewhere in docs that ts files are processed by ts-node after 1.19.0.

@leonardodino

Copy link
Copy Markdown
Contributor Author

Good one @mefcorvi (: Sorry for causing trouble.

I think we agree that having ts-node can help on the default scenarios, and overriding it when required by project needs.

I agree that it should be documented somewhere, especially as it's a recent change in a long-running project.

@mixed mixed mentioned this pull request May 8, 2019
remy added a commit that referenced this pull request May 25, 2019
* 'master' of github.com:remy/nodemon:
  chore: adding funding file
  chore: update stalebot
  feat: add message event
  fix: disable fork only if string starts with dash
  feat: add TypeScript to default execPath (#1552)
  fix: Quote zero-length strings in arguments (#1551)
@backbone87

backbone87 commented Jul 26, 2019

Copy link
Copy Markdown

can this either be properly documented or reverted? it cost me several hours to figure out why changing from: node -r ts-node/register ./src/index.ts to nodemon ... -r ts-node/register ./src/index.ts didnt work.

the ultimate reason is, that ts-node will be executed twice now and the 2nd one (from the cmd flag -r ts-node/register) will see the transpiled files already and complain that there are implicit types (because types are already compiled away by first ts-node instance from the implicit --exec ts-node).

another solution would be to check the arguments, when using implicit --exec ts-node, and remove any -r|--require ts-node/register[/transpile-only]

@HosseinAgha

HosseinAgha commented Aug 24, 2019

Copy link
Copy Markdown

@remy this broke lots of existing codebases that use -r ts-node/register in their dev script!
for the sake of google index :))
You should change nodemon -r ts-node/register src/main.ts to nodemon src/main.ts in your scripts if you get the following errors:
Cannot redeclare block-scoped variable 'tslib_1'
Cannot redeclare block-scoped variable 'name'
etc

@remy

remy commented Aug 24, 2019

Copy link
Copy Markdown
Owner

@HosseinAgha I raised another issues god knows when, saying that as I don't use typescript I really don't know what the right way of handling or fixing this. I've wondered for a long time if this change should be reverted (indeed asking so), but again, I don't use typescript so I've no idea.

Someone, please take a lead on what's right here and I'll happily merge in if it gets the consensus.

@backbone87

backbone87 commented Aug 25, 2019

Copy link
Copy Markdown

@remy the correct approach would be to only set the impicit --exec ts-node, if a) the script argument is a .ts file and b) there is no -r|--require ts-node/register[/transpile-only] in the options passed through to the executeable.

edit: alternatively just remove the whole implicit --exec ts-node and force explicit provision of any executeable other than node

@mrozekma

mrozekma commented Oct 2, 2019

Copy link
Copy Markdown

Thought I'd mention that this also breaks esm, as it has to be loaded before ts-node; the normal pattern when combining it with Typescript is:

node -r esm -r ts-node/register main.ts

Using nodemon instead of node here obviously doesn't work, and even if you take out -r ts-node/register it breaks because the ordering is wrong

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 this pull request may close these issues.

7 participants