aspect-build / rules_esbuild

Bazel rules for https://esbuild.github.io/ JS bundler
https://docs.aspect.build/rules/aspect_rules_esbuild
Apache License 2.0
27 stars 27 forks source link

[Bug]: rules_esbuild does not properly pass `deps` to js_binary #150

Open Aghassi opened 1 year ago

Aghassi commented 1 year ago

What happened?

I have an esbuild rule that I would assume is bundling a small config and it should have dependencies built into the output. However, on CI we found that the bundled config was missing the node_modules that were passed into the deps array for the esbuild rule call. See example code below

esbuild(
    name = "_syncpack.config.min",
    entry_point = "syncpack.config.js",
    # This file must always be CJS for syncpack to not ignore it
    # If it is not CJS, syncpack will silently fail and default
    # to it's own config, which will cascade into a failure on
    # our end since it will fail to exit 1 on CI
    format = "cjs",
    minify = True,
    # setting node as the platform will default us to CJS
    platform = "node",
    target = "esnext",
    deps = [
        "//arcanist/linters/linters/syncpack:node_modules",
    ],
)

bin.syncpack_lint_semver_ranges_binary(
    name = "lint-semver-ranges",
    args = [
        "--config",
        "$(execpath :_syncpack.config.min.js)",
    ],
    # this sets the tool to run from the root of the monorepo source tree
    chdir = "$$BUILD_WORKSPACE_DIRECTORY",
    data = [
        ":_syncpack.config.min.js",
    ],
)

This manifested in the config silently failing due to missing dependencies which I fixed by passing node_modules to the data array in the js_binary.

Note, the js_binary target is executed via bazel run

Version

Development (host) and target OS/architectures:

Output of bazel --version: 6.1.0

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file:

Language(s) and/or frameworks involved:

How to reproduce

Bazel run a target that takes a config built with esbuild as input and pass it to the runner. In this case, it's syncpack. Feel free to also reach out to me for more info.

Any other information?

No response

jbedard commented 1 year ago

Are those dependencies expected within the bundle or as runtime deps of the bundle?

Aghassi commented 1 year ago

Yes I expected it to be in the bundle generated by esbuild.

jbedard commented 1 year ago

Can you provide a simple example to reproduce it? What and how does syncpack.config.js import from syncpack:node_modules?

Aghassi commented 1 year ago

I can look into setting up a repro until we can find time to pair, but specifically the config imports fdir package which imports picomatch.

https://www.npmjs.com/package/fdir https://www.npmjs.com/package/picomatch

I declare both in the package.json that is used by the config and pass the :node_modules from that to esbuild to bundle it in.