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

[DATE]: Move the "node-windows is only supported on Windows" exception at execution time rather than at require time #297

Open
tlm-daniele-ricci opened this issue Sep 24, 2021 · 2 comments

Comments

@tlm-daniele-ricci
Copy link

Is your feature request related to a problem? Please describe.
It's hard to test scripts on Mac or Linux as we can't neither require the package.

Describe the solution you'd like
The only way I found to test my programs on Linux is to write a mock, ok, quite easy. When working on large program, probably it's not a problem, the mock could be useful for many other reasons.
But sometimes it happens that we have to work on short scripts. To write a mock to test a 30 lines script could mean double or triple the time to write the script...

Describe alternatives you've considered
So I think that letting us to require the package, to call new and to attach listeners even if not under Windows and throw the exception only when it's performed an operation that actually can be done only under Windows would facilitate the tests of short scripts also under Linux or Mac.

Additional context
A quick script to try to be a bit more clear.

// ATM neither this is possible under Linux or Mac, while it should be allowed
const nodeWindows = require("node-windows");

// This should be allowd under Linux and Mac
const { Service } = nodeWindows;

// Also this should be allowd under Linux and Mac
const service = new Service({});

// Both attaching a listener and referencing an impossible to call method should be allowd under Linux and Mac as well
service.on("install", () => service.start());

// This can't actually do nothing else than failing if under Linux or Mac
service.install();

Once said that, great job! Thank you very much!

Please use the reactions to cast a vote in favor of or against this feature suggestion »
@coreybutler
Copy link
Owner

This would require combining node-windows with node-mac and node-linux. The purpose of separating the libraries is to minimize the code shipped with production apps. For example, there is no point in shipping winsw.exe with macOS or Linux.

Testing a background service on each OS would still require the test to be run on each different operating system, so I'm not sure how much time this would realistically save. Even if it does, it will create bloated output for some operating systems.

I'll leave this open so others can vote on it.

@arturo-mendivil
Copy link

arturo-mendivil commented May 3, 2024

Agreed with this suggestion, my use case is not for tests, but sure it would be useful.

Currently we are using node-windows for a multiplatform electron app,
I was writing a factory class which implements node-windows service or node-mac service depending on the os, but i'm stuck due to this, i'm requiring both packages but get the error at the execution.
Would be great for our use case of service registering working for the same app in both OS.

Note:
As workaround I just removed the ifs at the start of node-windows and node-mac from my node_modules, but I don't want to be doing this every time when installing my dependencies.

// node_modules/node-windows/lib/node-windows.js
if (require('os').platform().indexOf('win32') < 0) {
  throw 'node-windows is only supported on Windows.';
}
// node_modules/node-mac/lib/node-mac.js
if (require('os').platform().indexOf('darwin') < 0){
  throw 'ngn-mac is only supported on Mac OSX.';
}

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

No branches or pull requests

3 participants