TKSS-Software / rodash

Lodash inspired Brightscript/ROPM utility for Roku apps
MIT License
4 stars 2 forks source link

trying to use rodash trigger an error #6

Closed ZeeD closed 2 years ago

ZeeD commented 2 years ago

Hi. first of all, thanks for your library! I am using it, and I'm writing a library myself. I think that there is an incompatibility between how the namespaces are handled by ropm and this:

in my library, I have installed rodash with ropm install rodash@npm:@tkss/rodash and I can access to the functions

everything works, except when I distribute my library to the users they are installing my library with ropm install mylibrary@npm:@myOrg/mylibrary, but then they find a little error:

src/source/roku_modules/tkss_rodash_v0/rodash.d.bs:371:90 - error BS1001: Cannot find name 'rodash_isNotInvalid'

 371  function get(aa as object, keyPath as string, fallback = Invalid as dynamic, validator = rodash_isNotInvalid as Function) as dynamic
 ___                                                                                           ~~~~~~~~~~~~~~~~~~~                        

I think this is because ropm install the traversed dependency with the full name tks_rodash, but in rodash there a refernce to rodash.isNotInvalid:

https://github.com/TKSS-Software/rodash/blob/104b2881bca1f9bcdcf7f97e33fabbae6c77f036/src/source/rodash/get.bs#L12

btw: I think this is the main reason on https://github.com/rokucommunity/ropm#overview the ropm teams has added the note

Don't define a baseline namespace. On install, ropm will prefix your package for you. (See Prefixes)

Could it be possible to remove the namespace rodash from the sources? it should be not necessary.

iObject commented 2 years ago

Thanks @ZeeD I will take a look at this

TwitchBronBron commented 2 years ago

@ZeeD I just looked into this with @iObject and what you are requesting is already being done. The node module distributes the code without any namespaces. image

And ropm install properly detects and renames those functions: image

And if I use an alias, it still works: image

So something else is at play here, either in your project or in the ropm transitive dependency resolution logic.

TwitchBronBron commented 2 years ago

@ZeeD Feel free to reach out to me in the RokuCommunity Slack Community? I'd be happy to help you privately (in a google meet or slack chat) since I believe you're working on a private project, right?

ZeeD commented 2 years ago

@TwitchBronBron thanks for the offer, and sorry I missed the invitation (I also use two different github handles, one for the private project and one for the rest of the world...) I tried to create a minimal scenario on https://github.com/ZeeD/transdeps

ZeeD commented 2 years ago

Hi @TwitchBronBron @iObject sorry to bug you!!! is the example I created clear? do you have the error yourself? can we look at it to understand what's happening?

TwitchBronBron commented 2 years ago

Hey, sorry I haven't had a chance to review your sample yet. I'll try to take some time today to look at it.

TwitchBronBron commented 2 years ago

@ZeeD I am able to reproduce the issue while using your sample project. Thanks for setting that up! I'll look into this and let you know how it goes. I'm closing this issue, as I believe it's actually a problem with ropm and has nothing to do with rodash.

TwitchBronBron commented 2 years ago

@ZeeD I just discovered that there IS a slight bug in the rodash code. We remove all the namespace prefixes before publishing (i.e. rodash_isNotInvalid -> isNotInvalid). However, we weren't doing that inside the type definitions, so it was incorrectly published with this code:

function get(..., validator = rodash_isNotInvalid as function)

when it should have been

function get(..., validator = isNotInvalid as function)

However, there's still a bug in ropm that would convert isNotInvalid back into rodash_isNotInvalid in that typedef file at ropm copy time anyway...so still need to fix that yet.

9 should resolve the rodash side of the issue.

TwitchBronBron commented 2 years ago

@ZeeD I believe this issue has been resolved. We released @tkss/rodash version 0.0.21 and ropm v0.10.10 which should resolve this issue. Give it a try and let us know how it goes!

ZeeD commented 2 years ago

Hi, sorry I was away from work. Yeah, I can confirm you that now everything works! Thank you so much!