Add rollup_config rule

[ashi009]

🚀 feature request

Relevant Rules

rollup_bundle

Description

We use 8 different rollup plugins to generate the final bundle, and some of these plugins require project-specific settings. It's difficult to maintain multiple rollup configs with minor differences.

Describe the solution you'd like

Add a rollup_config rule to generate config and capture the dependencies it needs and let rollup_bundle rule to include the deps of config_file.

rollup_config(
  name = "bundle_config",
  template = "//some/shared:template.rollup.js",
  substitutions = {
    "replaces": struct(
      "process.env.NODE_ENV" = "'production'",
    ).to_json(),
  },
  deps = [
    "@npm//@babel/preset-env",
    "@npm//babel-plugin-transform-jsbi-to-bigint",
    "@npm//core-js",
    "@npm//regenerator-runtime",
    "@npm//rollup-plugin-babel",
    "@npm//rollup-plugin-commonjs",
    "@npm//rollup-plugin-node-resolve",
    "@npm//rollup-plugin-replace",
    "@npm//rollup-plugin-strip",
    "@npm//rollup-plugin-terser",
  ],
)

rollup_bundle(
  name = "bundle",
  config = ":bundle_config",
  entry_point = ":index.ts",
  deps = [":app"],
)

It's also possible to wrap rollup_config in some macros to document config generation options, eg.

def rollup_config(name, replaces = {}):
  _rollup_config(
    name = "bundle_config",
    template = "//some/shared:template.rollup.js",
    substitutions = {
      "replaces": struct(**replaces).to_json(),
    },
    deps = [
      "@npm//@babel/preset-env",
      "@npm//babel-plugin-transform-jsbi-to-bigint",
      "@npm//core-js",
      "@npm//regenerator-runtime",
      "@npm//rollup-plugin-babel",
      "@npm//rollup-plugin-commonjs",
      "@npm//rollup-plugin-node-resolve",
      "@npm//rollup-plugin-replace",
      "@npm//rollup-plugin-strip",
      "@npm//rollup-plugin-terser",
    ],
  )

Describe alternatives you've considered

What we do right now is to wrap rollup_bundle with a macro that generates the config, then let rollup_bundle use that and injects rollup plugins to the deps. Under the hood it uses a genfile rule to generate config with ctx.actions.expand_template. However, this only works for a single config template given the tightly coupled rules and macro.

[alexeagle]

Cool!

I'd like to understand why the alternative macro approach doesn't work first. Why does it only work with a single config? Can't you call the macro twice and get two different generated configs?

[ashi009]

Cool!

I'd like to understand why the alternative macro approach doesn't work first. Why does it only work with a single config? Can't you call the macro twice and get two different generated configs?

The macro adds plugins as implicit dependencies to rollup_bundle, so I don’t need to repeatedly writing them all over the places. In case another config needs a different set of plugins this will just break.

Of course, I can leave that to the end user, but that’s just boilerplates and error prune.

[alexeagle]

this only works for a single config template given the tightly coupled rules and macro

that's the part I'd like to understand better. what are you able to do with a rollup_config rule that you couldn't do without it? I imagine you'd write a macro that you call from everywhere in place of rollup_bundle, which lists all the plugins you commonly depend on, and does the config expansion with replacements.

We do this a bunch in Angular https://github.com/angular/angular/blob/master/tools/defaults.bzl#L307-L325

If it's easy enough to do in user-space I'd rather document how to make the macro than need to maintain another rule.

[ashi009]

The ng_rollup_bundle is very similar to what we have ATM and this is indeed do-able from user-space entirely.

There are some caveats with the current macro approach:

• Conceptually, the config file is part of the build tool. Mixing the dependencies of the tool with what that tool builds feels wrong and confusing. (probably not so confusing for people with lots of npm experiences.)

  Our wrapper explicitly asks users to put plugins in a separate argument to rollup_bundle.

• There is no ready-to-use rule for config template expansion (at least for now, Expand template rule bazelbuild/bazel-skylib#191), and it's very likely to be the case. Without a rule to do that will force regular users to maintain multiple config files. Even with a generic template expansion rule provided, we will still need to do a second template expansion for inserting stamp data to the config -- which sounds redundant and inconvenient.

• Writing a custom rollup_bundle rule is rather difficult, it needs at least one rule for config template expansion and a macro to combine them, which I believe most users would like to avoid. The alternative is even more difficult, one needs to write something like ng_rollup_bundle and call rollup_bundle internals (which is not available in the new rollup_bundle).

I agree that adding a rule is overkill, but it will help to make the build file more declarative and readable. Moreover, given that we are going to add more node base tools in the future, having a pattern to deal with those configs and plugins would make life easier for both rule maintainers and consumers.

[alexeagle]

I had been hoping that overriding the config would be mostly possible with command-line flags, that's the simplest model. But I assume you have to vary your rollup config in ways the CLI doesn't permit.

What if rules_nodejs had a generic config_file rule that just does replacements? I've also been imagining a case where you don't even have a template file, and want to configure the tool entirely within the BUILD.bazel file. I keep deciding against that, because you should always use a file that your editor understands (you'll have a better time with blah.babelrc getting syntax highlighted than with a multiline starlark string literal) but perhaps there is a use case for that also.

If you had config_file then the macro for your rollup usecase would be pretty simple right?

Just thinking:

# I guess it would go in the builtin package?
load("@build_bazel_rules_nodejs//:index.bzl", "config_file")
config_file(
    // produces an output file named the same as the rule
    // so users don't need to guess at whether to depend on ":config" or ":config.js"
    name = "rollup_config_all_starlark.js",
    content = """
// Not sure it's a good idea to put things here instead of a file in the project but maybe in some cases?
export const {
    setting: "yes"
}
""",
)

config_file(
    name = "rollup_config_probably_better.js",
    template = "rollup_config_with_stuff",
    replacements = {
        "TMPL_yes": "no",
    },
)

Note that terser_minified already has a built-in way to turn your config into a template, maybe it should be pulled out into this config_file rule?

We need some more feedback about whether it's a good API.

[ashi009]

We have packed babel and terser in rollup action directly. So there are just too many configs for the rollup CLI to take. This is because that babel needs to be run before bundling the code, as it may add/remove imports. If the config for babel is different, all the following steps won’t benefit from caching fine grind staging files. Therefore doing them all together. I like the latter api, though I’d love to attach plugins as deps to it (perhaps add a provider just for that purpose?) The former API takes us back to the age before repository_rule is landed, that we have to put BUILD file content in BUILD file, which is just awkward. The replacement part should be able to use ctx.var, E.g. COMPILATION_MODE. Our genfile rule uses “$(VAR)” for that purpose. Perhaps do the stamping there as well? I could write up a small green doc if needed so we can put more use case there and discuss further. Alex Eagle <[email protected]>于2019年11月2日 周六上午12:08写道：

I had been hoping that overriding the config would be mostly possible with command-line flags, that's the simplest model. But I assume you have to vary your rollup config in ways the CLI doesn't permit. What if rules_nodejs had a generic config_file rule that just does replacements? I've also been imagining a case where you don't even have a template file, and want to configure the tool entirely within the BUILD.bazel file. I keep deciding against that, because you should always use a file that your editor understands (you'll have a better time with blah.babelrc getting syntax highlighted than with a multiline starlark string literal) but perhaps there is a use case for that also. If you had config_file then the macro for your rollup usecase would be pretty simple right? Just thinking: # I guess it would go in the builtin package? ***@***.***_bazel_rules_nodejs//:index.bzl", "config_file") config_file( // produces an output file named the same as the rule // so users don't need to guess at whether to depend on ":config" or ":config.js" name = "rollup_config_all_starlark.js", content = """ // Not sure it's a good idea to put things here instead of a file in the project but maybe in some cases? export const { setting: "yes" } """, ) config_file( name = "rollup_config_probably_better.js", template = "rollup_config_with_stuff", replacements = { "TMPL_yes": "no", }, ) Note that terser_minified already has a built-in way to turn your config into a template, maybe it should be pulled out into this config_file rule? We need some more feedback about whether it's a good API. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1314>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFH2B3MAQDS65KW4UCPJ2DQRRIALANCNFSM4JGLLFZA> .

-- Sent from Gmail Mobile

[ashi009]

Just discovered another use case. I'm trying to write a plugin in typescript for babel and want to use that in rollup_bundle. I use a ts_library rule to build the plug-in and include that as a dep in rollup_bundle. However, as the deps are mixed, so only es6 outputs are linked to node_modules. Therefore, rollup cli fails to run the code. 🤷‍♂️

[alexeagle]

Hi there, found this issue while searching for "stamp" as I think I've cleaned that up now.
I also just added tsconfig generation to ts_project so I have some fresh understanding of how config generation could work.

But, I'm still not convinced from reading through this again that there's a problem with rollup_bundle today which isn't neatly solvable in user-space. Do you still want this, and do you mind giving a more concrete example of the problem with current rules_nodejs?

[ashi009]

@alexeagle This is still a problem -- having rollup plugin built with ts_library won't work in rollup_bundle without some using some filegroup in middle to take the cjs output.

Just discovered another use case. I'm trying to write a plugin in typescript for babel and want to use that in rollup_bundle. I use a ts_library rule to build the plug-in and include that as a dep in rollup_bundle. However, as the deps are mixed, so only es6 outputs are linked to node_modules. Therefore, rollup cli fails to run the code. 🤷‍♂️

[alexeagle]

That's a restriction of ts_library making it hard to get JS outputs, and would apply to any downstream tool. So I think a rollup-specific solution is the wrong place to fix that. ts_project is my replacement rule that I think is the principled answer.

[ashi009]

Perhaps, #2162 is the solution to this problem as well. Let rollup use the plug-in ts_library in its plug-in deps, which has a transition to build ts files to es5/cjs output. Then rollup will include them in its run files. The deps part will use a different transition to require esnext/esmodule output, and rollup runtime will use those files for bundling. Even the plug-in may share the same dependency with the code being bundled, there will be no conflict.

[github-actions]

This issue has been automatically marked as stale because it has not had any activity for 90 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]

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"