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

Allow for custom rollup config #234

Closed
wants to merge 15 commits into from

Conversation

Globegitter
Copy link
Contributor

Even though the default rollup setup is pretty good and should work for most use cases, I think there are also quite a few use-cases that would want to utilize further rollup plugins (us included) and having to write a custom rule for each of these separate use-cases seems a bit cumbersome. So I came up with a way of providing a custom config without having to write your own rule.

Here one would have to provide their own node_modules, which is a superset of the existing node_modules (have not found a satisfying way to have the nodejs_binary depend on two sets of node_modules). Then one would have to copy the existing rollup.config.js and ensure to keep the TMPL_ variables in place. It is not quite ideal but imo stil makes it easier than creating a custom rule.

This would allow it to quite easily fix #216 and commonjs support mentioned in #137

Also internally we are experimenting with a few different plugins and this makes it easier for us than having to write/modify a custom rule.

@Globegitter Globegitter changed the title Custom rollup config Allow for custom rollup config Jun 18, 2018
Copy link
Contributor

@kalbasit kalbasit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR worked really well for #216, I'm able to run my app in a browser, replaces our gulp/webpack build.

@Globegitter
Copy link
Contributor Author

@kalbasit nice, happy to hear that

@kalbasit
Copy link
Contributor

@alexeagle This PR makes it easier to use Rollup, is it something you would consider, or is there some other way being worked on or considered?

@kalbasit
Copy link
Contributor

@Globegitter can you please rebase your PR over master? It looks like one file is conflicting.

@alexeagle
Copy link
Collaborator

New rollup_bundle rule lets you pass rollup.config.js. Should launch with next release

@alexeagle alexeagle closed this Sep 20, 2019
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Oct 17, 2020
…azel_rules_typescript_tsc_wrapped_deps//:node_modules

BREAKING CHANGE:
If you don't have an npm dependency on `@bazel/typescript`, you may be missing libraries like protobufjs which are required at runtime.
Run `yarn add -D @bazel/typescript` or `npm install --save-dev @bazel/typescript` to fix.

Closes bazel-contrib#234

PiperOrigin-RevId: 206037532
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Oct 18, 2020
…azel_rules_typescript_tsc_wrapped_deps//:node_modules

BREAKING CHANGE:
If you don't have an npm dependency on `@bazel/typescript`, you may be missing libraries like protobufjs which are required at runtime.
Run `yarn add -D @bazel/typescript` or `npm install --save-dev @bazel/typescript` to fix.

Closes bazel-contrib#234

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

Successfully merging this pull request may close these issues.

How to include node builtins in rollup bundle
3 participants