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

feat: add TypeScript to default execPath #1552

Merged
merged 1 commit into from May 1, 2019

Conversation

leonardodino
Copy link
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
Copy link

stale bot commented Apr 24, 2019

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
Copy link
Owner

remy commented Apr 24, 2019 via email

@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
Copy link

teebot commented May 6, 2019

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
Copy link
Owner

remy commented May 6, 2019

@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
Copy link

teebot commented May 6, 2019

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
Copy link

mefcorvi commented May 6, 2019

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
Copy link
Contributor Author

leonardodino commented May 6, 2019

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
Copy link

teebot commented May 6, 2019 via email

@leonardodino
Copy link
Contributor Author

leonardodino commented May 6, 2019

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
Copy link

mefcorvi commented May 7, 2019

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
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
Copy link

backbone87 commented Jul 26, 2019

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
Copy link

HosseinAgha commented Aug 24, 2019

@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
Copy link
Owner

remy commented Aug 24, 2019

@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
Copy link

backbone87 commented Aug 25, 2019

@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
Copy link

mrozekma commented Oct 2, 2019

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.

None yet

7 participants