[FR]: Standardized support for non-relative import paths of libraries #706
Comments
|
The difficultly with a standardized absolute import path in the Node.js ecosystem is that if it is not something handled by Node.js, then each tool would have to be taught how to understand it which is not scalable. Even if some support could be added to Node.js for a standard import mapping, the JavaScript ecosystem is already starting to migrate to tools build with golang (esbuild) & rust (swc, turbopack, rome) for better build times so those tools would also have to be taught how to understand the mapping as well. The simplest way to make standardized imports that all tools understand is to link 1p packages into node_modules with pnpm workspaces. You eluded to this with your reference to Here, for example, first party packages under the |
|
@gonzojive does that make sense as being out-of-scope for rules_js? We don't want to invent a new semantic that's Bazel-specific. |
The support I'm after already appears to be part of tsconfig.json through the "paths" attribute, which is respected by esbuild and tsc. rules_js would probably need to modify the tsconfig.json fed to those tools to populate paths appropriately.
It's understandable. I will probably end up writing wrappers around the rules to make this easier, though. Also, I'm not sure of a good strategy for how to write proto rules on top of vanilla rules_ts. What would you do? To control the import path of the protos, it seems each js_proto_library would need to create an NPM package, and clients would depend on that NPM package somehow.
Thanks for the link. I will try this out more. I find it pretty confusing. Some notes:
|
|
Some notes to myself on how to implement the It seems the way the linked example works relies on explicit In my ts_proto_library(
name = "clock_proto_ts",
proto = "clock_proto",
import_path = "@protos/my_app/clock_pb",
deps = [
# Dependency on some other ts_proto_library that
# carries information about how protocol buffer language
# imports corresponds to TypeScript language imports.
"//x/y/z:timestamp_proto_ts"
],
)Presumably an Edit 1: I'm guessing I'll need to generate both an $ bazel query --output label_kind "//:node_modules/inspirational-quotes"output: So _ts_proto_library_rule(
name = "clock_proto_ts",
proto = ":clock_proto",
ts_import_path = "@protos/my_app/clock_pb",
output = "clock_proto_ts_generated.ts",
)
npm_package(
name = "clock_proto_ts_npm_package",
package = "@protos/my_app/clock_pb",
srcs = [
"clock_proto_ts_generated_package.json",
"clock_proto_ts_generated.ts",
],
deps = [
# deps extracted from "//x/y/z:timestamp_proto_ts",
]
)
npm_link_package(
# Should name be "node_modules/@protos/my_app/clock_pb"?
# https://github.com/search?q=npm_link_package&type=code
name = "WHAT GOES HERE??",
src = "clock_proto_ts_npm_package",
auto_manual = False,
)Users of the library would then depend on Edit 3: Except the npm_link_package above doesn't work when it's not in the root of workspace. So there would be no way for the ts_proto_library macro to generate an appropriate call to npm_link_package. Aside: the documentation for the |
|
See pnpm-workspaces/apps/alpha/src/main.ts in my attempt to get a prototype of the above to work. It seems pretty standard to put all the npm_link_package calls in the root of the repository. Why is that? I would prefer for a library just depend directly on an npm package declared in some random directory. (This is how dependencies work in most languages for Bazel.) If every npm package needs to be declared in the root, it doesn't scale well for a big monorepo and requires editing two BUILD files for every new library. |
|
@gregmagolan and I are discussing what feels like a missing documentation page on "linking". By the time you've gotten to the API doc for "Linking" is an npm concept https://docs.npmjs.com/cli/v8/commands/npm-link rules_js doesn't require that you link dependencies, as you observed you can also just use TS pathmapping support (which was originally added to the language for google3 !). I'm not sure what you mean
since the tsconfig.json file is written by the user and not generated. We plan to add a TypeScript + Proto example and possibly a rule, and have discussed design, but so far don't have any funding to work on that. Instead of writing your own, maybe you could donate a feature bounty so we could provide one for everyone? |
Working on it.
What is stopping the node_modules trees from being constructed based on the deps of any particular target? Could something like this be supported? # Suppose this is at <repo-root>/a/b/BUILD.bazel
npm_package(
name = "bazely_lib",
package = "@foo/bar",
srcs = [
"main.js",
],
package_json = "my_package.json",
)
js_library(
name = "script",
srcs = ["script.js"],
deps = [
":bazely_lib",
],
)When invoked on bazely_lib, the esbuild rule (or tsc rule) would want a directory structure like Which it seems like it could construct by analyzing the deps of "script". |
|
The node_modules tree is made up of output artifacts (tree artifacts & symlinks) that make up the symlinked node_modules trees. There is quite a bit of complication in the npm link rules to construct these trees, but in theory the syntax sugar you have in your example is possible and the links could be automatically added to the same package as the target (rules cannot output to other bazel packages) for any npm_package targets in the deps with the caveat that The virtual store requirement does make the magic of this syntax sugar less attractive since it doesn't stand on its own. may be less clear than in terms of what you're actually depending on, which is node_modules links. |
|
Thanks for all the explanations. Hopefully this helps others, too. To be clear, to make the snippet from #706 (comment) work today, something like the following npm_package(
name = "bazely_lib",
package = "@foo/bar",
srcs = [
"main.js",
],
package_json = "my_package.json",
)
npm_link_package(
name = "node_modules/@foo/bar",
src = ":bazely_lib",
root_package = package_name(),
)
js_library(
name = "script",
srcs = ["script.js"],
deps = [
":node_modules/@foo/bar",
],
)I suppose what I don't see is why the Could rules_js be modified to support depending directly on npm_packages? Rules like the esbuild one could parse dependencies of type "npm_package" while preparing the input files for the tool action. Presumably that would involve changes to js_lib_helpers.gather_files_from_js_providers: # excerpt from esbuild rule...
input_sources = depset(
copy_files_to_bin_actions(ctx, [
file
for file in ctx.files.srcs + filter_files(entry_points)
if not (file.path.endswith(".d.ts") or file.path.endswith(".tsbuildinfo"))
]) + other_inputs + node_toolinfo.tool_files + esbuild_toolinfo.tool_files,
transitive = [js_lib_helpers.gather_files_from_js_providers(
targets = ctx.attr.srcs + ctx.attr.deps,
include_transitive_sources = True,
include_declarations = False,
include_npm_linked_packages = True,
)],
)
launcher = ctx.executable.launcher or esbuild_toolinfo.launcher.files_to_run
ctx.actions.run(
inputs = input_sources,
outputs = output_sources,
arguments = [launcher_args],
progress_message = "%s Javascript %s [esbuild]" % ("Bundling" if not ctx.attr.output_dir else "Splitting", " ".join([entry_point.short_path for entry_point in entry_points])),
execution_requirements = execution_requirements,
mnemonic = "esbuild",
env = env,
executable = launcher,
) |
|
Setting the I'm not sure what the failure mode would be as I've never tried multiple virtual stores. It is not something that you can do with pnpm itself so to go down that route will require some care. Supporting it might be possible but it would likely require some non-trivial refactoring of the code and lots of testing & new test cases & e2e tests. |
|
The other side-effect that the "auto-link" approach would hit is when targets depend directly on npm_packages in other packages. The only choice is to link to the package of the target and make another virtual store there since you have no way to know if the package is linked to a virtual store elsewhere. This will end up with duplicate 1p packages in multiple virtual stores. This might be fine but it could be surprising to some users and come with unexpected side-effects. |
|
I do see a possibility of this simplification working if deps could be shared between multiple virtual stores (this is a pre-req since otherwise deps have to be duplicated in each one) and if npm_package always linked a virtual store in the package the target is in. If possible, it would be a few days of exploratory work and if it there were no blockers a few days to a week to implement and the auto-link could work. |
|
I'm trying to follow the comments. Understanding the answers to some of the questions below might help clarify. When running a tool like esbuild, it seems the working directory for the tool is the Is there any benefit to copying into a directory specific to the target being built? For example, when compiling The custom could be to output to and then execute esbuild with working directory |
|
Bazel restricts a target such as The pattern of re-rooting the entire output tree for a target into an output folder such as With regards to the BINDIR, the cwd for rules_js actions is set to the BINDIR of the target platform. Bazel expects a build target is expected to produce its outputs in that bazel-out tree. The tool being run & its runfiles can be in another platform output tree altogether such as the exec platform and this works as expected. I haven't seen any issues with different bindirs as Bazel handles putting all input files to a target in the bindir for the target platform being built and expects its outputs to go into that tree as well. Hopefully that helps clarify some of the constraints rule sets have to work with. |
Is it the copying of files that doesn't scale or something else? It seems like tools already go over the entire list of input files, e.g., That function is already O(direct_deps + transitive_deps). There are currently O(direct_deps) copy_files_to_bin_actions, while in the re-rooted version there would be O(direct_deps + transitive_deps) actions. I would expect shared node_modules directories could still be created. Symlinks to those directories would need to be created in the re-rooted version of the output tree. In any case, it sounds the current system is sufficient based on this comment:
For this example # liba/BUILD.bazel
npm_package(
name = "pkga",
package = "@myapp/a",
deps = [
"//libc:pkgc",
]
)
# libb/BUILD.bazel
npm_package(
name = "pkgb",
package = "@myapp/b",
deps = [
"//libc:pkgc",
]
)
# liba/BUILD.bazel
npm_package(
name = "pkgc",
package = "@myapp/c",
)
# cmd/BUILD.bazel
js_library(
name = "script",
srcs = "script.js",
deps = [
"//liba:pkga",
"//liba:pkgb",
]
)Would you generate something like this? |
We do already copy all source files to the output tree so that node tools only have to deal with a single file tree instead of separate source & output file trees (also touched on here). This makes rules_js much more compatible with node tools at the expense of some overhead; although even in this case multiple targets that reference the same source file can share the same source file output artifact so this ends up as If each target had its own unique tree in the output tree, that tree would have to include copies of source files & copies of all other inputs & npm packages so that node_modules resolution would work within that unique tree all the relative paths were valid in there as well. This is more of
Close. The virtual store is put into a You'd get something like this: with the symlinked node_modules virtual store tree handling transitive deps |
|
I'm running into a similar annoyance writing a macro that calls def ts_proto_library(name, proto, visibility = None, deps = [], tsconfig = None):
"""A rule for compiling protobufs into a ts_project.
Args:
name: Name of the ts_project to produce.
proto: proto_library rule to compile.
tsconfig: The tsconfig to be passed to ts_project rules.
"""
# ...
# This is how implicit dependencies on npm packages would work if rules_js
# operated the way Go does. However, as written this doesn't work because
# if ts_proto_library(name = "foo") is called in bazel package @bar//baz,
# the @com_github_gonzojive_rules_ts_proto//:node_modules directory
# is not in the set of directories searched by nodejs to resolve "google-proto".
implicit_deps = [
"@com_github_gonzojive_rules_ts_proto//:node_modules/@types/google-protobuf",
"@com_github_gonzojive_rules_ts_proto//:node_modules/google-protobuf",
]
deps = [x for x in deps]
for want_dep in implicit_deps:
if want_dep not in deps:
deps.append(want_dep)
ts_project(
name = name,
srcs = [
ts_files,
],
assets = [
non_ts_files,
],
deps = deps,
tsconfig = tsconfig,
)How would you deal with these npm dependencies? Some options I see
ts_proto_library(
name = "polyline_ts_proto",
proto = ":polyline_proto",
visibility = ["//visibility:public"],
deps = [
"//location:location_ts_proto",
"//:node_modules/google-protobuf",
"//:node_modules/@types/google-protobuf",
],
)
workspace(
name = "myorg",
)
load("@com_github_gonzojive_rules_ts_proto//ts_proto:workspace_deps.bzl", "install_rules_ts_proto")
install_rules_ts_proto(
npm_deps = {
"myorg": {
"google-protobuf": "//:node_modules/google-protobuf",
"@types/google-protobuf": "//:node_modules/@types/google-protobuf",
},
"dep1": {
"google-protobuf": "@dep1//:node_modules/google-protobuf",
"@types/google-protobuf": "@dep1//:node_modules/@types/google-protobuf",
},
}
)I'm not sure if the npm_deps needs to be per-repository or not (looking into it now). I'm under the impression that each repository needs its own |
install_rules_ts_proto now takes a dictionary mapping npm package name to bazel
targets required to satisfy that package dependency. Example:
```starlark
install_rules_ts_proto(
dep_targets = {
"google-protobuf": "//:node_modules/google-protobuf",
"@types/google-protobuf": "//:node_modules/@types/google-protobuf",
},
)
```
This is required because ts_project and similar targets cannot depend directly
on NPMs; they must instead depend on "linked" npm targets within the target's
bazel package or one of its parent packages. This is discussed in
aspect-build/rules_js#706 (comment).
|
Latest release of rules_ts shouldn't depend on google-protobuf anymore after @thesayyn refactoring of the worker code |
|
"google-protobuf" is used above an example of how tricky it is to work with
NPM deps in JS/TS libraries across different repos (seemingly). It could be
replaced by any library.
A further example is if I have a library "foo" in repo "a" that depends on
an npm package "bar." Should the library depend on npm packages linked in
repo "a" (such as @a//:node_modules/bar)? If that foo library is used as a
dep in repo "b", which also links package "bar", it seems two copies of the
bar library get instantiated, which presumably is bad because of things
like multiple values of singleton variables.
|
|
Gotcha. I think I have to read through the threads above to catch up on the context. Generally speaking, each |
|
I wonder if import maps are relevant to this discussion. They are mentioned in the nodejs docs. There is some discussion of this in issues filed against the TypeScript and nodejs projects: microsoft/TypeScript#43326 |
|
Interesting. Looks like node already supports a variation of that it calls "subpath imports", https://blog.spencersnyder.io/solving-the-long-relative-paths-problem-natively-in-node-js-6aeebc3a81bd... but I don't see how to make that play nicely with type checking or bundling. It's a difficult problem space because there is such a wide array of tools in the javascript ecosystem. Even if rules_js could populate the "paths" of a tsconfig, that wouldn't help if you were bundling with rollup or esbuild. Whatever bundler you're using would also need to be aware of the paths as will node or the browser at runtime if you're not bundling away the paths. Consuming through pnpm workspaces adds overhead and boilerplate but at least most tools agree on standard node_modules resolution. If you use pnpm workspaces you can avoid the additional |
What is the current behavior?
As far as I know, JavaScript doesn't standardize a way to associate a fully qualified import path with a module. (Unlike Go and Java.) However, having a canonical, non-relative import path for libraries is useful, and JS-related tools find a way to support this.
Different tools (node, TypeScript, browsers?) seem to have different ways of being configured to handle "absolute" imports. For example,
tsconfig.jsonhas apathsoption that can be used. esbuild I believe supports thetsconfig.jsonpaths.It is currently up to the rules_js user to manipulate the configuration objects passed to bazel-run tools (esbuild, tsc, node, ...) and the IDE such that global imports work correctly.
Describe the feature
Should associating an absolute import path with a module or set of modules be given some sort of standardized support by rules_js?
I'm looking for something like rules_go's import_path arg. I want to have a TypeScript file in a project called "lib.ts" (with corresponding js file "lib.js"). I would like to import "lib.js" using an import path of my choosing, such as
"@proj/foo/lib".@proj/foomay have no relationship to my workspace name.My actual use case is so I can generate protobuf code that is imported like
import {Timestamp} from "@protos/google/type/timetamp_pb.js"using a build rule likets_proto_librarywould be either a rule or a macro. One of the outputs of the rule would be something that keeps track of this global "@protos/google/type/timestamp_pb" module name and its association with some generated JS/TS files. When tools are called, they would be made aware of the import mappings within all transitive dependencies.I'm not sure the recommended way to do this is, if it is not recommended, or what. Perhaps there is a way to do this using
npm_package.Fund our work
The text was updated successfully, but these errors were encountered: