rokucommunity / ropm

A package manager for the Roku platform.
MIT License
31 stars 5 forks source link

ropm rename the tasks names, but not the references to them #56

Closed ZeeD closed 1 year ago

ZeeD commented 1 year ago

Let's say I am writing a library that contains a Task like

<?xml version="1.0" encoding="utf-8"?>
<component name="MyTask" extends="Task">
    <!-- ...stuff... -->
</compoent>

that is meant to be used by my library internally. I can reference it by name like

myTaskName = "MyTask"
' ... stuff
myTask = CreateObject("roSGNode", myTaskName)

(and it works) But then, if I publish and use the library I have written I have ropm that has (rightfully) changed the content of the xml with the prefix of my library (so now I have a Task named mylibrary_MyTask in /component/roku_modules/mylibrary/... BUT internally my code still refers to MyTask

I tried to follow the suggestion in https://github.com/rokucommunity/ropm#ropm_prefix-source-literal (witch if I understood well is for the name of the callback of the task, not the name of the task itself, but still...) and tried adding a ROPM_PREFIX but now bsc just throws some errors at me (so I cannot even prepare the package)

.../src/source/taskrunner.bs:64:47 - error BS1001: Cannot find name 'ROPM_PREFIX'

 64              myTask = CreateObject("roSGNode", ROPM_PREFIX + myTaskName)
 __                                                ~~~~~~~~~~~                    

Is there something I can do to make ropm aware that it should set myTaskName = "mylibrary_MyTask"?

ZeeD commented 1 year ago

mmm, ok, I just added a ' bs:disable-next-line comment to allow the usage of ROPM_PREFIX for bsc; and then ropm install mylibrary did the trick and replaced ROPM_PREFIX in the right way. still... if I may, it feels more like a workaround than a "appropriate solution"

TwitchBronBron commented 1 year ago

Just to clarify, the actual problem was that when BrighterScript encounters errors, it refuses to transpile the code. We have a few things that could improve the situation:

ZeeD commented 1 year ago

I kinda agree that this problem should be fixed modifying bsc and not ropm (I still feel that a solution could be found tuning how ropm treats the task names "transparently" for the rest of the tools)

Coming back at bsc, I am wondering on what kind of way do you want to "register" these globals (just a configuration field in bsconfig.json? something else?) but I concur that it may be the better of the three choices.

FWIW: in my case it is a bit complicate because I want 1) to package, publish, install and use my library in an application (so I need that ROPM_PREFIX is written in the .brs files in the package 2) to deploy my library directly with my unit tests (so not only I need bsc to compile my sources, but also to remove the ROPM_PREFIX reference, otherwise the .brs code is not valid.

I already use two different bsconfig.jsons, so I leverage that and I found a "solution" like

TwitchBronBron commented 1 year ago

Another option is to actually define the prefix as empty string before you use it. This would allow you to use the same codebase in both ropm and direct deployments.

Like this:

ROPM_PREFIX = ""
myTask = CreateObject("roSGNode", ROPM_PREFIX + myTaskName)

You can see in this ropm unit test that this pattern is supported.

We should probably add this style to the documentation.