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

Problem with Newley released types #2206

Comments

@KevinGruber
Copy link
Contributor

  • Versions: [email protected], [email protected]
  • nodemon -v: 3.1.1
  • Operating system/terminal environment (powershell, gitshell, etc): MacOs, terminal
  • Command you ran: Via api in typescript

Expected behaviour

When importing nodemon there is a default export for functionality defined in the types.

Actual behaviour

With the addition of #2204 there where types added to the project, but no default export types for functionality -> that leads to a not usable nodemon in typescript

Steps to reproduce

import nodemon from 'nodemon';

-> nodemon is not a module


@remy
Copy link
Owner

remy commented May 28, 2024

Just checking, but does require work? And can you import using `import * as nodemon from 'nodemon'?

@KevinGruber
Copy link
Contributor Author

const nodemon = require('nodemon'); - WORKS doesn't load types

import * as nodemon from 'nodemon'; - DOESN't work

I am pretty sure it is because there is no export of the functionality of nodemon.

Based on the types nothing is exported

@remy
Copy link
Owner

remy commented May 28, 2024

I need to have a play. I only added types as an attempt to help people who were requiring the module, but I don't personally use TypeScript (some context). If you have any advice, please do share, otherwise I'll tinker this week 👍

@remy
Copy link
Owner

remy commented May 28, 2024

Okay, to clarify, import nodemon from 'nodemon' does work, i.e. this code works:

// test.mjs
import nodemon from 'nodemon'
nodemon({}).on('start', () => {
  console.log('started');
});

But the types aren't being found by tools like VS Code.

This is my code:

package.json

{
  "name": "2206",
  "version": "1.0.0",
  "main": "index.js",
  "dependencies": {
    "nodemon": "^3.1.1"
  }
}

index.js (the sub process):

console.log('Hello, World!');

And the run script: test.mjs

import nodemon from 'nodemon';

nodemon({}).on('start', () => {
  console.log('started');
});

From the command line:

SCR-20240528-jwdv

@KevinGruber you're saying it's undefined, can you share your code?

@KevinGruber
Copy link
Contributor Author

Ok sry for the misunderstanding.

The runtime code works yes, but the typescript doesn't build because of "wrong" types.

undefined in the sense of types in TS

Beforehand it worked because there where no types (and I configured ts to allow for no types).

If you want I can make a PR with adjusted types?

@remy
Copy link
Owner

remy commented May 28, 2024

Yeah, if you could do a PR I'd appreciate it. I don't want to convert the project to typescript, but I do want to expose the types to help devs (which is why I've just added index.d.ts)

@KevinGruber
Copy link
Contributor Author

I have the types already working, I now ran into the problem that they are not fitting the api approach types.

Can you help me and send me a list of allowed attributes? Like script etc.

are those than just merged with the current NodemonSettings? Or is NodemonSettings just the json config representation?

@remy
Copy link
Owner

remy commented May 28, 2024

This:

NodemonSettings just the json config representation

Sod, there's more props, I'll take a look and see if I can get a complete list. Annoyingly the --dump command doesn't quite list it out, because the inbound config is rearranged to the internal config (like script is moved to execOptions.

@remy
Copy link
Owner

remy commented May 28, 2024

If you have a branch, send up a work in progress PR and I can add to it (I think…)

@remy
Copy link
Owner

remy commented May 28, 2024

Found the following - pretty sure that's it.

script?: string; 
ext?: string; // "js,mjs" etc (should really support an array of strings, but I don't think it does right now)
events?: { [key: string]: string }; 
env?: { [key: string]: string };
exec?: string; // node, python, etc
execArgs?: string[]; // args passed to node, etc

Copy link

🎉 This issue has been resolved in version 3.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@arpowers
Copy link

@remy the types still aren't correct in latest version, type errors happen when using the documented API (the API that was 100% working prior to 3.1.1.

@KevinGruber
Copy link
Contributor Author

@arpowers what do you mean with working before, it wasn't using any types before

Can you show me the error?

@remy
Copy link
Owner

remy commented May 29, 2024

@arpowers a screenshot would be useful here, or othewise some code for context. Ta.

@arpowers
Copy link

Sure here are the errors that show up after upgrade:

Let me know if you want a link to the code.

@fiction/core/plugin-env/restart.ts:19:13 - error TS2503: Cannot find namespace 'nodemon'.

19     config: nodemon.Settings
               ~~~~~~~

@fiction/core/plugin-env/restart.ts:65:8 - error TS2339: Property 'on' does not exist on type '(settings: NodemonSettings) => Nodemon'.

65       .on('log', () => {})
          ~~
Screenshot 2024-05-29 at 1 37 13 PM

@arpowers
Copy link

arpowers commented May 29, 2024

I should mention that I'm using "@types/nodemon": "^1.19.6", as well, let me try and uninstall that and see if anything changes.

....and no change, type errors are still there.

@remy remy reopened this May 29, 2024
@remy
Copy link
Owner

remy commented May 29, 2024

Oooh. I know what it is. Though, not sure how you represent it in Typescript. It's because the nodemon function returns itself, and it's supposed to be a singleton.

This isn't a fix, but if you chain the. on to the call to nodemon, I suspect it will work.

I'll see if I can fix the types in the morning, or if anyone gets a PR before then, happy to merge.

Unrelated: I wonder who is maintaining @types/nodemon...that isn't mine... 🤔

@arpowers
Copy link

no hurry, just locked version for now.

@remy
Copy link
Owner

remy commented May 30, 2024

@KevinGruber or @arpowers I'd appreciate some eyes on my change: https://github.com/remy/nodemon/pull/2208/files - I've tested it and it appears to work (with autocomplete), but more importantly the code actually still runs.

Again, I'm not a regular TS dev, so wary it could be wrong.

@remy remy added the has PR label May 30, 2024
@KevinGruber
Copy link
Contributor Author

@remy yes that works for me I tested both use cases

I wasn't aware that you can use nodemon as a singleton like that. Sry for missing that :-(

I noticed another thing yesterday can you provide string and string[] to nodeArgs and execArgs?

I took your suggestion and it took only string[] but my old code provided string.

If it supports both and is expected I would change the type to string | string[]

@remy remy closed this as completed in #2208 Jun 3, 2024
remy added a commit that referenced this issue Jun 3, 2024
Copy link

github-actions bot commented Jun 3, 2024

🎉 This issue has been resolved in version 3.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment