search

bazelbuild / rules_postcss

PostCSS rules for Bazel
Apache License 2.0
10 stars 13 forks source link

Convert examples/ and tests/ into sub-workspaces #48

Open rzhw opened 3 years ago

rzhw commented 3 years ago

This PR resolves the problem of rules_postcss dependents pulling in Sass and TypeScript because of our examples, even if they won't actually use Sass or TypeScript.

The way this PR chooses to resolve the problem is by turning examples/ and tests/ into sub-workspaces, where the Sass and TypeScript dependencies are added.

Solving this problem using sub-workspaces also allows us to create new examples with npm deps that won't affect the root workspace.

(For example, Tailwind is a highly popular PostCSS plugin for UI development, with 25.7k stars on GitHub—for reference, PostCSS has 23k stars at time of writing. If it was decided to merge a Tailwind example, then merging this PR followed by merging such an example wouldn't affect downstream users of rules_postcss.)

Closes #43

nex3 commented 3 years ago

This is well outside my Bazel expertise. Maybe @alexeagle should take a look at this one as well?

alexeagle commented 3 years ago

My last commit on rules_python shows how nested example workspaces can be integration tested. It also detects breaking changes where users WORKSPACE files would need to change due to a dangling dev dependency. I don't think tests should be a nested workspace though. They are cumbersome. Just exclude test folders from your distribution artifact.

rzhw commented 3 years ago

It seems like the approach from https://github.com/bazelbuild/rules_python/pull/337/files and https://github.com/bazelbuild/rules_python/pull/338/files is quite hefty, particularly the setup of .bazelrc + tools/bazel_integration_test/update_deleted_packages.sh + tools/bazel_integration_test/bazel_integration_test.bzl.

Could you expand on the nested test workspace being cumbersome? It seems to me like making tests a nested workspace is much lighter in comparison to the rules_python approach; Bazel CI works as expected, and the primary disadvantage is that local testing requires developers to cd into the tests dir. Though the cd issue could potentially be alleviated using a package.json script, which would be not entirely surprising in a Node.js environment.