Skip to content

Ability to stop propagation of link_node_modules aspect information #2941

@devversion

Description

@devversion

The link_node_modules aspect is currently used for capturing mappings that the linker can consume. The aspect is applied on the target relying on the linker (such as a nodejs_test). It would be beneficial to have an API that allows for a target to skip the module mapping collection. This is useful if a target applies an outgoing transition and therefore results in transitive dependencies being built under a different output directory (compared to bazel-out).

This is problematic because a test target for example cannot rely on the target applying the transition (e.g. a pkg_npm target), as well as on the non-transitioned original dependency (such as the ts_library used for the NPM package). e.g.

Error in fail: conflicting mapping at 'test': ':core'
  and ':core' map to conflicting 
   bazel-out/x64_windows-fastbuild/bin/packages/core and
   bazel-out/x64_windows-fastbuild-ST-2e5f3376adb5/bin/packages/core

As it can be observed above: There are two conflicting mappings because core dependency on the
pkg_npm_wrapped target is also considered as linker mapping for the test. This is unexpected and it
would be fixable if there was a way for the pkg_npm_wrapped/ng_package rule to return an empty
set of mappings / or disable the propagation of the e.g. deps attribute. The mappings from
the transitive dependencies are not desirable anyway because the NPM package is supposed to be the
mapping if at all.

ts_libary(
  name = "core",
  deps = [..],
  package_name = "@angular/core",
)

# This is a wrapper-implementation of `pkg_npm` that has an outgoing transition
# applied on "deps". The ":core" target will be built with different build settings.
pkg_npm_wrapped(
  name = "core_npm_package",
  deps = [":core"],
)

######

ts_library(
  name = "test_lib",
  deps = [":core"],
)

jasmine_node_test(
  name = "test",
  data = ["core_npm_package"],
  deps = [":test_lib"]
)

Workaround tried:

I have tried working around it by manually setting the link_node_modules__aspect_result property on the NPM package
rule. e.g.

    return struct(**{
        "files": depset([package_dir]),
        "link_node_modules__aspect_result": {}
    })

This does not work because the link_node_modules aspect does not check if the property is already set and Bazel will error because an already-existing "provider" is re-assigned. A solution would be to check for the presence in the aspect before. This will allow for advanced cases like described.

Ideally the link_node_modules aspect should not use the deprecated? struct approach for providers to begin with. Then it should check for the presence of the provider before trying to set it in the aspect (allowing for a solution like in the snippet above). This will allow for advanced use-cases. Alternatively, the aspect could look for an attribute on the target it visits. Though I like the idea of having manual control over mappings.

I'm happy to send a PR. Might send a proposal tomorrow.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions