Rules_nodejs: Support for entry points relative to current BUILD file directory

[rzhw]

Currently, users provide the full path to a local js file to use it as an entry point, including the workspace name. It's a bit of extra typing, but it's workable.

This gets a bit more cumbersome for nodejs_binary using entry points that aren't at fixed locations e.g. from other rules in macros. (FWIW this isn't an issue for rules e.g. ctx.workspace_name + "/" + ctx.file.foo.path, but it's unrelated because rules can't call other rules.)

Below is an example: we have an intermediary rule which outputs a js file, and we want the nodejs_binary to use it as an entry point. But the macro doesn't know the path to the js file, so we have to get the user to specify the path.

def foo_rule(
        name,
        # "workspace_name/path/to/BUILD/directory/"
        current_path,
        visibility = None):

    # Intermediary rule outputs "{name}.js".
    intermediary_name = "%s.intermediary" % name
    intermediary_rule(
        name = intermediary_name,
    )

    # Nodejs rule combines current path with known output name
    nodejs_binary(
        name = name,
        data = [
            "@//:node_modules",
            intermediary_name,
        ],
        entry_point = "%s%s.js" % (current_path, intermediary_name),
        visibility = visibility,
    )

Proposal

Add the current path to the node module resolver's list of resolve paths.

The above entry point can be changed to a relative path:

    nodejs_binary(
        # ...
        entry_point = "%s.intermediary.js" % name,

Caveats

Files supplied in data would clash with node_modules. Convoluted example:

    nodejs_binary(
        # ...
        data = [
            "@//:node_modules",
            "some_node_module/foo.js",
        ],
        entry_point = "example.js",
    )

const foo = require('some_node_module/foo.js'); // Which one?

Alternatives considered

Change entry_point to be attr.label instead of attr.string

Ruled out because this would break references to node_modules.

Split entry_point into two attributes, one attr.label for local references, one attr.string for node_modules references

Not as straightforward for users.

Split nodejs_binary into two rules, one for local entry points, one for node_modules entry points

Not as straightforward for users.

[alexeagle]

Sorry it took me so long to reply, and thanks for sending a PR for this.

I'm concerned about the namespace collisions, as you point out. I think the entry_point needs some specific syntactic hint that we want to load from the package. This could be as simple as starting with ./

Note that any build file can use the https://docs.bazel.build/versions/master/skylark/lib/native.html#package_name and repository_name functions to do the same thing as ./, but it's tricky because repository_name could be empty or it could be @name, which needs a bit of logic to convert to a path.

You also suggested allowing a label in the entry_point - this is already supported by bazel with special $(location :label) syntax. That's the most explicit way to do this, so I gave it a try:
https://github.com/bazelbuild/rules_nodejs/pull/42

what do you think about that approach?

[rzhw]

Thanks for the response! Apologies about the equally late reply :stuck_out_tongue:

Great points, and I agree the namespace collision is a major downside of this suggestion. Also wasn't aware of those native.* functions, cool! Shame they can't really be used, so the $(location :label) syntax SGTM; I'll reply to the linked PR.

[nictuku]

Hi. Right now, folks I talked to are having trouble figuring out how to use entry_point, probably because the instructions are unclear and there is no support for $(location).

@alexeagle seems to be working on support $(location :file.js) for the entry_point. Any ETA for when that's going to land? Thanks!

[alexeagle]

Instead of using location or rootpath helpers, we should change entry_point to be a label.
https://github.com/bazelbuild/rules_nodejs/pull/42#issuecomment-421635378

[alexeagle]

Another place this is a problem
https://github.com/angular/angular/blob/f8096d499324cf0961f092944bbaedd05364eea1/packages/core/test/bundling/animation_world/BUILD.bazel#L22-L28