Using protected instead of private

[MakeShiftArtist]

trafficstars

Describe the bug Wampy (@types/wampy) does't have proper typescript support for many methods and properties. The module also doesn't allow the class to be extended due to many properties and methods using private instead of protected. Protected does the same thing as private, except allows the class to be extended, and allows the extended class to modify those protected properties. While this isn't an issue for most, the websocket I'm using needs to modify the wampy instance just slightly. For reference, this is my WampyClient in NodeJS (working, but many linting errors, which cause typescript to throw errors without ts-ignore comments)

To Reproduce Steps to reproduce the behavior:

1. Create a new TypeScript project
2. Install both wampy and @types/wampy
3. Import the wampy class
4. Extend the wampy class
5. Notice that this.options doesn't allow proper onChallenge or onConnect
6. Notice that you're unable to edit properties like Wampy._protocols due to it being private instead of protected

Expected behavior Due to the WebSocket I'm using, I need to modify Wampy's default behavior. Due to everything being private instead of protected, I'm forced to add mulitple // @ts-ignore comments because TypeScript is unaware of many properties and methods.

Screenshots

Environment (please complete the following information):

• OS: Windows
• Node Version: 17.2.0
• Wampy Version: 6.4.2
• @types/wampy Version: 0.0.3

Additional context While it makes sense to leave many of these properties or methods private, there are use cases where modifying them is required. A native way to modify these values would also be suitable, so if there is one to achieve what I'm doing without extending Wampy, please let me know. I believe this is the only method to get wampy to work with my specific websocket. Please take a look at the full file if you need help understanding what's happening. (there may be a few bugs as I'm currently refactoring the project in TypeScript and haven't had the chance to update it. Thank you in advance!

Side note I know this isn't going to have perfect TypeScript support. But it's currently very limited on the support it does have due to these restrictions. Callback function types for onConnect and onError don't have any args, which causes errors to be thrown when trying to get args passed into those callbacks in TypeScript.

[KSDaemon]

Hi @MakeShiftArtist ! Thanks for report! Well, indeed, typings for wampy are outdated. I have plans to rewrite wampy in ts but it will take time. For now — you can help — update type definisions, we can place them also here in repo so we can update them in future. Your help is very appreciated!

[MakeShiftArtist]

@KSDaemon I'm going to make a pull request with type support for Wampy when I'm finished, but I wanted to ask first if it was okay if I changed all @private members to @protected members because even with updated types, I can't patch wampy to work with my websocket due to them being private instead of protected.

Edit: Didn't mean to close this issue

[KSDaemon]

Great! Well, yes, protected is ok!

[MakeShiftArtist]

I actually decided to avoid using JSDOC all together and instead do a full rewrite in TypeScript. It's about 98% done, I just gotta test to make sure it is still working properly and remove a bunch of comment bloat that is unnecessary in TypeScript as many of the comments are just type annotations.

Edit: All seems to be working correctly, but I can't run proper tests on my machine. If you clone my repo, or merge it with a TypeScript fork, you may be able to get tests working on your machine.

[MakeShiftArtist]

@KSDaemon I have a question about a specific property. The _ws property is typed with JSDOC as an Object, but it's created as a constructor. In getWebSocket it's shown as having 5 parameters, where every WebSocket client I can find has either 2 or 3. Is this a mistake? If not, could you point me to where the WebSocket client it is expecting is?

[KSDaemon]

Regarding getWebSocket and 5 params: wampy works in both: node.js and browser environments. In node there is no standard ws object with same api as in browser. But, there is a small lib websocket, you can see it in dependencies, that gives the same API as in browser. And if you check its constructor, it has a few options.

[KSDaemon]

Well, interesting, what the problems do you have running test? What cmd do you run? npm run test:node-no-crossbar should work without any real wamp routers

[MakeShiftArtist]

Regarding getWebSocket and 5 params: wampy works in both: node.js and browser environments. In node there is no standard ws object with same api as in browser. But, there is a small lib websocket, you can see it in dependencies, that gives the same API as in browser. And if you check its constructor, it has a few options.

Reviewing the constructor, the extra args aren't passed directly, but within an Array as the 3rd param like so

new W3CWebSocket(requestUrl, requestedProtocols, [[[[origin], headers], requestOptions], clientConfig])

In my typescript rewrite, I will patch this

As for testing, I will have to check again when I'm back at my computer

[MakeShiftArtist]

@KSDaemon The npm run test error

npm run test

> [email protected] test
> npm run test:node && npm run test:browser


> [email protected] test:node
> NODE_ENV=test nyc ./node_modules/mocha/bin/mocha --exit --require @babel/register -R spec

'NODE_ENV' is not recognized as an internal or external command,
operable program or batch file.