Proposal: resolve node module dependencies during bsc (ropm-style)

elliot-nelson commented 1 year ago

BACKGROUND

We're attempting to build a Rush monorepo, which uses PNPM workspaces with symlinks for node_modules.

We can have "application using libraries" using built-in bsc file mapping functionality, for example:

Folder Structure (click to show)

```text apps roadrunner/ src/ source/ Main.brs /libraries/ dynamite/ src/ source/ Dynamite.brs components/ Dynamite.xml images/ explosion.gif ```

bsconfig.json (click to show)

```json { "files": [ "manifest", "source/**/*.*", "components/**/*.*", "images/**/*.*", { "src": "../node_modules/@acme/dynamite/src/source/**/*.*", "dest": "source/dynamite" }, { "src": "../node_modules/@acme/dynamite/src/components/**/*.*", "dest": "components/dynamite" }, { "src": "../node_modules/@acme/dynamite/src/images/**/*.*", "dest": "images/dynamite" } ] } ```

The roadrunner app can now use the dynamite library in a reasonable way, but there are some caveats:

No automatic re-namespacing, like ropm -- you have to make sure each individual library doesn't introduce any conflicts with each other.
If you want App depending on Library A depending on Library B, the App needs to list all the remaps for all the libraries (including transitive dependencies). That's not ideal!

PROPOSAL

What if bsc itself could detect and remap node modules folders.

For example, I imagine this syntax: <package-name>:/path/to/file (<package-name> replaces pkg).

The desired behavior would be that in my source folder, I might type:

import "@acme/dynamite:/source/Dynamite.bs"

Then brighterscript would automatically do a few magical things:

First, it understands that @acme/dynamite prefix means ./node_modules/@acme/dynamite/
Inside that folder is a src/ folder, and in there is source/Dynamite.bs.
Now that we've found the file, we need to automatically remap it
- It should act just like we've added an entry to bsconfig.json, even though we haven't
- We could mimic ropm: source/roku_modules/acme_dynamite/Dynamite.bs

With this scheme, transitive dependencies are automatically handled!

If LibraryA depends on LibraryB, then the same logic will handle it without issue:

e.g.: ./node_modules/LibraryA/src/source/LibraryA.bs depends on LibraryB:/source/LibraryB.bs
During compile it knows this file is at ./node_modules/LibraryA/node_modules/LibraryB/src/source/LibraryB.bs
The final output is:
- source/roku_modules/LibraryA/LibraryA.brs
- source/roku_modules/LibraryB/LibraryB.brs

What about other types of URLs, like images and resources?

Because this new capability is a URI prefix, it can be used for any source code that can reference pkg:, like references to images or JSON files!

Once again, a reference to @acme/dynamite:/images/explosion.gif should be modified to pkg:/images/roku_modules/dynamite/explosion.gif, and the file copied to that location in the output.

What about components?

This is where I need the most help -- I'm not sure how I can reference components.

I have one idea, which is a variation on the above: maybe if you import anything using a package reference, it automatically detects and generates the mappings for all subfolders, including components.

In this case, if any .bs file imports "@acme/dynamite:/source/anything.bs", that would trigger the entire package (every subfolder it might have, like images, resources, components) to be copied into the appropriate roku_modules structures, even if you don't reference them.

(I'm open to other approaches.)

Namespacing?

All of the above really nicely solves the copying-and-remapping mechanic, and gives you an experience that's close to TypeScript's require or import. But, it doesn't solve the namespacing problem, e.g.: you have two libraries that raw-define function add(). How can you "namespace" the code using the library name?

I think we could take ropm's approach and default to prefixing every function with acme_dynamite_ (etc.). If these names got long, then in theory we could support custom remapping in bsconfig:

{
    "packages": {
        "@acme/dynamite": {
            "namespace": "Dynamite"
        }
    }
}

You shouldn't need to list transitive dependencies here, because unless you are actually calling them, you won't need to know what they are named and so custom remapping isn't required.

Next steps

I realize this is a wall of text, hoping that I've explained what we are trying to do (work within the symlinking / pnpm folder structure and not use ropm's "copy files" approach). Very open to poking holes in the design and additional discussion!

TwitchBronBron commented 1 year ago

I think most of this could be accomplished with a bsc plugin. Plugins can call the program.setFile() function to add files from any srcPath and give them any dest/pkgPath. You can also modify the code on its way in, so there's a possibility of doing the namespacing Like this:

const srcPath = path.resolve("../node_modules/@acme/dynamite/src/source/lib/someLib.brs");
const fileContents = fsExtra.readFileSync(srcPath).toString());
const prefixedFileContents = applyRopmPrefix(fileContents);

program.setFile({
    src: srcPath,
    dest: "source/dynamite/lib/someLib.brs"
}, prefixedFileContents);

Images and resources are not currently loaded into the bsc program, but soon they will be .

I'm definitely open to figuring out a path forward for this in brighterscript, if it makes sense. However, if it could be a bsc plugin, that would also be reasonable. I've been thinking for a while that having an official ropm plugin would be really useful. We could validate ropm packages on their own, and also ingest ropm packages into bsc projects where it handles all the "symlinking" without actually having to run the symlinking. It kind of seems like there's a good deal of overlap between those concepts and your asks here.

elliot-nelson commented 1 year ago

Yes, 100%, there's a lot of overlap :).

In my head, originally, what I wanted was symlinking in ropm.

The issue there is, what do you symlink to? If you symlink src/source/roku_modules/LibraryA to ./node_modules/LibraryA/src/source, in a separate step from bsc, that does immediately fix all the source problems. But, you have to build the whole monorepo with conflicts in mind -- every Library has to make sure it defines its own namespace, or watch for global function collisions.

Part of what I'm trying to do is figure out how to avoid "magic" e.g. instead of globally knowing that maestro is mv, if you somehow had to have in your file:

import mv from 'maestro';

I think this just isn't realistic because of the presence of XML components which all just appear and you need to manually prefix, I assume.

elliot-nelson commented 1 year ago

As I play around with some example source code more, I've made some adjustments here...

DESIGN ATTEMPT 2

There are two discrete problems: one, collecting up all the files we need and organizing them into folders, and two, making sure the code (function names etc.) make sense.

PROBLEM 1

Let's invent a new, small JSON snippet: the package mapping. It might look like this:

{
    "ropm": {
        "@acme/dynamite": "Dynamite",
        "maestro": "mv"
    }
}

This snippet could be added to bsconfig.json, or possibly to package.json itself. The property is named ropm because this mapping tells bsc that it wants "ropm-style" copying of the named packages into the folder structure of the build output. (We'll ignore the value-part for now, that's for Problem 2.)

What's important about this snippet is that it is recursive. If AppA depends on LibraryA depends on LibraryB, bsc will respect this mapping wherever it finds it, collecting up all references to all packages.

The output of this algorithm can be boiled down to a list of package-mappings for each package (to use in Problem 2), and a list of "file mappings" that bsc can handle as usual.

PROBLEM 2

To explore Problem 2, think about LibraryA and LibraryB that both depend on maestro.

LibraryA maps maestro to mv, and has a bunch of references in code to mv.Task etc. LibraryB maps maestro to maestro, and refers to maestro.Task instead.

AppA depends on both libraries, but doesn't use maestro itself at all. The job of brighterscript is to:

Take all packages that refer to "maestro", and pick a single prefix to use for this compilation. Let's say it picks e8da1c.
It needs to un-namespace (reverse ropm algorithm) all code in LibraryA (stripping mv), and then re-namepsace (applying ropm algorithm) with e8da1c.
Identical operation with LibraryB, replacing maestro with e8da1c.
It now namespaces (applies ropm algorithm) the maestro package itself, using prefix e8da1c.
In the final output, roku_modules folders contain LibraryA/, LibraryB/, and e8da1c/, and both LibraryA and LibraryB refer to files in e8da1c.

(The prefix chosen here is intentionally pointless; the real algorithm could pick one or the other, but it has to not conflict with any namespaces AppA may have created, and should work even if it is an obscure and pointless prefix.)

I'm not sure whether all of this is possible in just a plugin today, but I think this is roughly what we need.

elliot-nelson commented 1 year ago

I think, as much as possible, I'd want to reuse the existing ROPM tool.

I've been reviewing the source for the tool today and it really does a lot of the above... here's my notes on what I'd want to change:

Specifying prefixes for packages

In https://github.com/rokucommunity/ropm#renaming-modules, it suggests using package.json:

{
    "dependencies": {
        "promise": "npm:roku-promise@1.2.3"
    }
}

This is not possible in a Rush monorepo because you need to use the specifier for in-repo projects:

{
    "dependencies": {
        "roku-promise": "workspace:*"
    }
}

So, we need some other place to specify our desired prefixes. I suggested packages in bsconfig in my earlier post; but any place in bsconfig or package.json would work.

Copying files from the source to destination

In ROPM the copying magic happens in https://github.com/rokucommunity/ropm/blob/master/src/prefixer/RopmModule.ts#L145. But, we want the logic and not the actual copy (because we're going to let the destination be the build output folder, not the src folder).

I think with a little reorganization and a couple more public API methods, we could call right into ropm to "scan and generate the files list", then take over the rest.

elsassph commented 1 year ago

Wouldn't the solution to bring 1st class support for ropm in bsc? e.g. that you could get to a point where producing the roku_modules folders physically isn't needed? This really only is needed for vanilla BRS.

elliot-nelson commented 1 year ago

Wouldn't the solution to bring 1st class support for ropm in bsc? e.g. that you could get to a point where producing the roku_modules folders physically isn't needed? This really only is needed for vanilla BRS.

Skip the plugin entirely? 🤔

I think if we could show that the changes I'm proposing are strictly better and have no downsides compared to the original method, moving it into bsc itself would be nice. An advantage to a plugin, at least at first ("bsc-pnpm-ropm" or "bsc-modern-packages" or something) is that anyone currently used to the ropm install && bsc method wouldn't be blind-sided by changes to the behavior.

elsassph commented 1 year ago

No, having ropm as a bsc plugin, instead of an installation task. It may be what you're suggesting already - in which case I agree with you :D

rokucommunity / brighterscript