Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pkg_npm() targets as dependencies for ts_library() #1967

Closed
nkeynes opened this issue Jun 22, 2020 · 4 comments
Closed

Support pkg_npm() targets as dependencies for ts_library() #1967

nkeynes opened this issue Jun 22, 2020 · 4 comments
Labels
Can Close? We will close this in 30 days if there is no further activity

Comments

@nkeynes
Copy link

nkeynes commented Jun 22, 2020

🚀 feature request

Relevant Rules

ts_library() and pkg_npm()

Description

Currently, attempting to use a pkg_npm() target as a ts_library dependency results in an error of the form "<merged target //Foo/src:pkg> doesn't contain declared provider 'DeclarationInfo', even if the pkg_npm was originally itself generated in part from another ts_library() target. For example,
... foo/BUILD:
ts_library(
name = "foo_lib",
module_name = "@foo"
srcs = glob(["**/*.ts"])
)

filegroup(
name = "foo_static",
srcs = glob(["/*.json","/*.css"])
)

pkg_npm(
name = "foo",
package_name = "@foo",
srcs = [":foo_static"],
deps = [":foo_lib"]
)
... bar/BUILD:
ts_library(
name = "bar",
srcs = glob(["**/*.ts")
deps = ["//Foo:foo"]
)

Describe the solution you'd like

Ideally, pkg_npm() targets could be accepted as ts_library dependencies and treated the same as any imported node module. Our main use case for this is in libraries that bundle separate files (e.g. JSON, CSS, etc) along with TS code, where those files are resolved at runtime relative to the source files. Currently (pre-bazel) this is implemented by just copying all the additional files into the tsc build directory

Describe alternatives you've considered

I've tried to work around this by putting a direct dependency between the ts_library() targets, which is sufficient for compilation, but then there doesn't seem to be an obvious way to get the correct, merged pkg_npm dependency at runtime - having both as dependencies leads to mapping errors ala:
attribute deps: conflicting mapping at '//Foo/src:pkg': '@foo' maps to both ["execroot", "bazel-out/darwin-fastbuild/bin/lib/Foo/src".

@alexeagle
Copy link
Collaborator

I tried to do this in https://github.com/bazelbuild/rules_nodejs/pull/1898/files
The problem is that pkg_npm produces a directory artifact, so it can't also expose individual files in that directory as labels. That means it also can't expose providers that let it serve as a dependency to other rules.
Our plan is to generalize node_module_library which is currently used as a rule to expose third-party packages from npm. It's basically the shape we need to depend on first-party libraries too.

@github-actions
Copy link

github-actions bot commented Sep 8, 2020

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!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Sep 8, 2020
@alexeagle
Copy link
Collaborator

Decided that js_library is the right rule for this

@alexeagle
Copy link
Collaborator

See #2187

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? We will close this in 30 days if there is no further activity
Projects
None yet
Development

No branches or pull requests

3 participants