feat(rollup): add silent attr to rollup_bundle to support --silent flag #1680
Conversation
As rollup_bundle already supports specific rollup flags with specific attributes on the rollup_bundle rule, adding a boolean attribute for silent makes the most sense. This boolean determines if the --silent flag is added to the rollup command when called.
josephperrott
requested review from
alexeagle,
gregmagolan and
soldair
as code owners
|
Note: related to #1661 which was also for My take is: we wrap rollup in a custom rule, and attrs like This has an unfortunate meaning in rollup - |
| @@ -168,6 +168,9 @@ Otherwise, the outputs are assumed to be a single file. | |||
| cfg = "host", | |||
| default = "@npm//rollup/bin:rollup", | |||
| ), | |||
| "silent": attr.bool( | |||
There was a problem hiding this comment.
Could you add a warning in this doc that using silent can cause rollup to ignore problems in your code, and link to https://github.com/rollup/rollup/blob/master/docs/999-big-list-of-options.md#onwarn and maybe rollup/rollup#2686 ?
You could also mention that Bazel expects tools to print nothing on success, so enabling --silent gives a more Bazel-idiomatic experience, but the default here is False due to how it also drops warnings.
There was a problem hiding this comment.
Done, I added this information to the attr docstring.
| doc = """Whether to execute the rollup binary with the --silent flag, defaults to False. | ||
|
|
||
| Using --silent can cause rollup to [ignore errors/warnings](https://github.com/rollup/rollup/blob/master/docs/999-big-list-of-options.md#onwarn) | ||
| which are only surfaced via logging. Since bazel expects printing nother on success, setting silent to True |
As rollup_bundle already supports specific rollup flags with specific
attributes on the rollup_bundle rule, adding a boolean attribute for
silent makes the most sense.
This boolean determines if the --silent flag is added to the rollup command
when called.
I was unable to add tests for this as its change is to the stdout of the execution
of bazel, not the results of the rollup command.