Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

This node causes Node.JS module get-installed-path to stop working #97

Closed
TotallyInformation opened this issue Jan 22, 2019 · 6 comments
Labels

Comments

@TotallyInformation
Copy link

get-installed-path is used by node-red-contrib-uibuilder to find the install paths for front-end libraries that have been installed using npm.

If this node is installed, the sync version of the function getInstalledPathSync stops working. I've not tried the other version which returns a promise.

The issue lies at line 130 of that module:

const modulePath = filePaths.find((filePath) => {
...

The find function is never executed.

If node-red-contrib-opcua is removed, the function starts working again.

To me, this implies that something that this node is doing is overwriting the native Array.prototype.find function? Everything else appears to be working as normal.

@mikakaraila
Copy link
Owner

mikakaraila commented Feb 11, 2019

There is actually no exports and Array is not used so it could be overwrite it. I suppose it could come from some underlying dependency instead of directly node-red-contrib-opcua. Have to investigate more...
My first guess is collections package.
montagejs/collections#215

@TotallyInformation
Copy link
Author

Hi, thanks for the response. I don't actually use your node myself as I don't have the hardware but as you can see from the reference, one of the users of uibuilder reported it.

extends core types (e.g extends Array.prototype with additional non-enumerable properties like .set)

Urgh! Not a good approach! Sounds like you are right. This illustrates exactly why extending the prototype of a native object isn't a good approach. Too easy to break something.

@TotallyInformation
Copy link
Author

Though, I can see that the collections package is actually at v5, you are using v3. Possibly this has been fixed in a later release?

@mikakaraila
Copy link
Owner

Can you test it now? I am at work and I will have time perhaps later today?

@mikakaraila
Copy link
Owner

OK, I updated collections and node-opcua to latest ones.

@mikakaraila
Copy link
Owner

mikakaraila commented Feb 14, 2019

Has anybody tested new version? New one will use JS Map instead of Set, should be better now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants