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

feat(esbuild): support location expansion in esbuild args #2564

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

dae
Copy link
Contributor

@dae dae commented Mar 28, 2021

When an esbuild rule is passed an argument like "--inject:path/in/repo.js",
this works when building in the local workspace, as esbuild is invoked
with the correct working directory. But if the esbuild rule is in
a remote workspace (eg bazel build @other_workspace//path/in:esbuild_rule),
then the path is no longer valid.

By expanding $(location ...) references in provided arguments, it allows
callers of the rule to pass arguments like the following, which work
in both local and remote repo cases:

esbuild(args = ["--inject:$(location //path/in:repo.js)"], ...)

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

When an esbuild rule is passed an argument like "--inject:path/in/repo.js",
this works when building in the local workspace, as esbuild is invoked
with the correct working directory. But if the esbuild rule is in
a remote workspace (eg bazel build @other_workspace//path/in:esbuild_rule),
then the path is no longer valid.

By expanding $(location ...) references in provided arguments, it allows
callers of the rule to pass arguments like the following, which work
in both local and remote repo cases:

esbuild(args = ["--inject:$(location //path/in:repo.js)"], ...)
@mattem
Copy link
Collaborator

mattem commented Mar 28, 2021

Thanks! Wonder what's going on the circleci tests 🤔 @alexeagle Any ideas?

@alexeagle
Copy link
Collaborator

@dae you probably setup circleci on your fork of rules_nodejs? That causes circleci to trigger in the wrong place for PRs. If you remove that setting I think your PRs can go green.

@alexeagle alexeagle merged commit eb3bd7e into bazel-contrib:stable Mar 30, 2021
@dae
Copy link
Contributor Author

dae commented Mar 30, 2021

I have linked CircleCI to my GitHub account in the past, but don't use it currently, and don't recall ever doing so for rules_nodejs. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants