Rules_nodejs: ts_project should provide LinkablePackageInfo

[alexeagle]

Currently a ts_project needs a js_library or pkg_npm between it and another ts_project, if the latter wants to import it with a package name rather than a relative import.
We should investigate if that intermediate rule is really necessary.

[longlho]

So update on how I got this working:

Unfortunately js_library does not expose DeclarationInfo so it cannot be a dep to ts_project, same w/ pkg_npm. Therefore I basically copy-pasta js_library and add DeclarationInfo to it.

After that, in order for TSC to resolve it (since it's generated file), I added path resolver in root tsconfig.json:

{
    "paths": {
      "@mylib/*": [
        "bazel-out/k8-fastbuild/bin/packages/*",
        "bazel-out/darwin-fastbuild/bin/packages/*",
        "bazel-out/x64_windows-fastbuild/bin/packages/*",
        "bazel-out/k8-dbg/bin/packages/*",
        "bazel-out/darwin-dbg/bin/packages/*",
        "bazel-out/x64_windows-dbg/bin/packages/*"
      ],
    }
}

My custom rule:

def _ts_monorepo_subpackage_impl(ctx):
    ts_project = ctx.attr.ts_project
    ts_project_files = ts_project[DefaultInfo].files.to_list() + ts_project[DeclarationInfo].declarations.to_list()
    files = []

    is_all_sources = ts_project_files[0].is_source
    for src in ts_project_files:
        if src.is_source:
            dst = ctx.actions.declare_file(src.basename, sibling = src)
            copy_file(ctx, src, dst)
            files.append(dst)
        else:
            files.append(src)

    files_depset = depset(files)

    result = [
        DefaultInfo(
            files = files_depset,
            runfiles = ctx.runfiles(files = ts_project_files),
        ),
        ts_project[DeclarationInfo],
    ]

    path = "/".join([p for p in [ctx.bin_dir.path, ctx.label.workspace_root, ctx.label.package] if p])
    result.append(LinkablePackageInfo(
        package_name = ctx.attr.package_name,
        path = path,
        files = files_depset,
    ))
    return result

_ts_monorepo_subpackage = rule(
    _ts_monorepo_subpackage_impl,
    attrs = {
        "package_name": attr.string(
            mandatory=True,
            doc="Package name in package.json"
        ),
        "ts_project": attr.label(
            mandatory=True,
            doc="Label for ts_project"
        ),
    },
)

def ts_monorepo_subpackage(
        name,
        package_name,
        **kwargs):
    ts_project(
        name = "%s-lib" % name,
        **kwargs,
    )
    _ts_monorepo_subpackage(
        name = name,
        ts_project = ":%s-lib" % name,
        package_name=package_name
    )

At that point ts_monorepo_subpackage can be a dep of another ts_monorepo_subpackage

cc @cocreature @aherrmann @pyrocat101

[alexeagle]

Before seeing your reply here I was also playing with this to understand the current state, and found the same thing, that pkg_npm doesn't propagate the DeclarationInfo

https://github.com/bazelbuild/rules_nodejs/pull/1898 is my little repro.

This is pretty easy to fix but we should be careful to fix it in the right way since users will have lots of code that does this and we'll never want to change it.

• I think it's strange to have to do ts_project -> js_library, these feel like peers. Given the current design, js_library knows how to expose a "module_name" aka package name so maybe ts_project should also
• OTOH the package.json file (if there is one) is the Source of Truth about the name of the package, and we should always key off of that. pkg_npm should be aware of package.json (see my draft PR #1897) but I don't think ts_project or js_library should be aware of it. So you should totally be able to have ts_project -> pkg_npm(name = [package.json{name}]) -> ts_project.

This makes me think we should fix it in both ways:
1) pkg_npm should propagate the transitive depset of DeclarationInfo it found among deps (and also if srcs includes a .d.ts file it should be included)
2) ts_project should get a package_name attribute to behave like js_library

Trying to think through the implications of 2) though - if you have a js_library or ts_project with package_name=foo and then want to publish it to npm, you'll wrap with pkg_npm and now we will have two different sources of the foo package_name mapping. That might be a confusing sharp edge, if they conflict?

[longlho]

Hmm my solution fails when trying to chain subpackages import together, e.g @my/lib1 -> @my/lib2 -> @my/lib3 :(

I think (1) makes sense and (2) can optionally have a package_name? In terms of packaging preserving the name in package.json makes sense but we also want the ad-hoc ts-node like behavior of compiling 1-off build scripts and execute w/ nodejs_binary (thus not having package_name), which right now is done w/ ts_library but I'm not sure what the direction for this is w/ regards to ts_project?

[longlho]

ts_project -> pkg_npm -> ts_project also seems sensible to me.

[joeljeske]

It seems like your thought process could be geared toward the library author’s use case; for those that publish to packages to npm. I would like to speak up as one who has migrated to bazel but is not publishing to npm rather only building containers of apps for deployment.

I do not think that the package.json should be the primary method of specifying the module name for a ts_project, I think it does deserve its own package_name. Before bazel we were using rush.js (and before that lerna) and therefore each intermediate package had a package.json, but after migrating to bazel we found it freeing to be rid of that constraint: The classic npm package name is restricting! You can only have 1 slash; between scope and name. We found the scope of the project ends up not being all that informative, just a common name space that all our packages shared. I found it much better (albeit different from the node ecosystem) to be able to import a package using the pathname in the repo itself, or a pathname from a common root location. We found this more Intuitive. It was very freeing to also support nested packages, that are reflected in the package name as well. It has led to more intuitive organization and discovery of related packages.

All that to say, I really appreciated the flexibility of an arbitrary module_name of ts_library and think something just as flexible should be used for ts_project.

[longlho]

I'd advocate for either no package_name, or package_name from package.json otherwise it gets pretty confusing, so maybe a bool flag instead of a attr.string?

[DavidANeil]

We aren't currently using ts_project, but I can say pretty easily that if "add 1600 package.json to the codebase" was the requirement to use it, we would never use it.

[longlho]

I don't think that's what I'm suggesting. If you need to refer to a ts_project that's not the path from root workspace, you need a package.json. Otherwise regular root/path/to/a/package is fine. Having another package_name that's not 1 of those is a module name mapping nightmare for existing bundling tooling that ALREADY have their own aliases mechanism.

[alexeagle]

1897 includes a way for pkg_npm generate package.json if none is provided. Certainly won't require having one, especially because of the first-party-use-only use case

[alexeagle]

@gregmagolan helped me remember a design flaw with pkg_npm exposing typings. pkg_npm produces a directory output (TreeArtifact in Bazel terms) which is opaque - bazel rules can't see what files are inside, you can only address the label of the directory. The linker is okay with this if you take the whole directory as an input.

However DeclarationInfo has to take a depset of files, so ts_project uses it by adding individual .d.ts files to the inputs. So the TreeArtifact that came from pkg_npm isn't an input and the linker fails to symlink it within the execroot.

However the fact that a first-party pkg_npm doesn't behave like a third-party node_module_library is disconcerting. We should think about this some more.

[longlho]

In the mean time are there any recommendations to solve this issue?

[alexeagle]

in our sync meeting we decided to go ahead with ts_project having its own way to declare the package_name, similar to js_library
https://github.com/bazelbuild/rules_nodejs/blob/c87c83fbee3241d05e4c43a0c5f1bd4a5088ead9/internal/js_library/js_library.bzl#L79-L83

optionally, we could copy a package.json file across to the bin folder, but it feels like that should be the job of pkg_npm

[alexeagle]

@longlho does this do what you need? https://github.com/bazelbuild/rules_nodejs/pull/1924

[longlho]

yup it does! :)

[alexeagle]

Discussion in team meeting: we think we want a single rule to be the hub in a hub-and-spoke model, so that we don't end up sprinkling assets and package_name in a bunch of places.
node_module_library is the existing rule with the closest semantics - we should leave pkg_npm as a code-reorganization and publishing mechanism without generally having dependencies on it.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

[alexeagle]

We have js_library now

[alexeagle]

Going public API with it: https://github.com/bazelbuild/rules_nodejs/pull/2187