dgp1130 / rules_prerender

A Bazel rule set for prerending HTML pages.
14 stars 0 forks source link

Investigate alternative CSS bundlers #46

Closed dgp1130 closed 1 year ago

dgp1130 commented 2 years ago

Currently we use rules_postcss for bundling CSS files, however it's current support level seems to be in a weird place (see https://github.com/bazelbuild/rules_postcss/issues/73/ and https://github.com/bazelbuild/rules_postcss/issues/74/). rules_prerender doesn't really have strict requirements here beyond resolving @import and the way rules_postcss does this is unnecessarily complicated IMHO. It would be worth investigating Parcel or ESBuild as an alternative CSS bundler which might be simpler, more modern, and more maintainable.

rules_nodejs has a Parcel example, though this seems intended for bundling JavaScript, not CSS.

Also see https://github.com/dgp1130/rules_prerender/issues/34/ for a separate investigation into ESBuild for bundling JS. Using the same tool for both pieces of the build could have other maintainability advantages.

dgp1130 commented 2 years ago

Tried using @parcel/css-cli via Bazel but quickly found that I couldn't actually execute it. It turns out that this package is implemented entirely natively and doesn't actually export any JavaScript, instead it uses a postinstall to download native binaries. npm_package_bin() doesn't work here and even trying to directly execute the binary via a genrule() doesn't work. Ultimately rules_nodejs sees the 'bin': { 'parcel-css': 'parcel_css' } in the package.json and generates a nodejs_binary() for it even though there's no JavaScript to execute.

I imagine this is still solvable as ESBuild works the same way but is directly supported in @bazel/esbuild. Taking a quick look they seem to retrieve this binary via a toolchain which gets pulled directly from NPM (not via npm install AFAICT).

The easier solution for now was to use @parcel/css which exposes a Node library for Parcel rather than a standalone binary. This still wraps the WebAssembly, but with a Node API which can be directly imported. I made my own nodejs_binary() which composed @parcel/css and it worked reasonably well. I was able to update css_binaries() to use Parcel and bundle its inputs to resolve and inline all imports.

The main challenge I haven't solved yes is that I had to use relative CSS imports for now for Parcel to resolve files correctly. Even that likely only works because the two files I was importing happen to both be source files. If one was generated, it would exist in a different artifact root and Parcel wouldn't know to look there. Workspace-relative imports also wouldn't work. Parcel does seem to support custom resolvers (example), so this could likely be solved. I'll have to experiment a little more.

Current progress is at ref/parcel.

dgp1130 commented 2 years ago

I took a deeper look at a custom resolver, however it seems that @parcel/css doesn't actually support that. I'm unclear how .parcelrc custom resolvers work if not through this package, but it's possible their dependencies and interactions don't line up the way I think they do. I filed an issue to clarify my understanding and offered to contribute the feature if it is a reasonable ask.

dgp1130 commented 2 years ago

In the @parcel/css issue I was directed at using the Parcel API from @parcel/core. I spent some time hacking on that and was able to eventually get something which worked.

Loading and calling the bundler was relatively straightforward. Unfortunately there doesn't seem to be a good way of defining an explicit output path, so I used an in-memory filesystem for Parcel and then manually forwarded the result to the real filesystem. I think it may be possible to get Parcel to do the right thing without this trick, but this should be good enough for now.

The biggest challenge was loading a resolver plugin. This turned out to be quite involved as plugins are defined in the .parcelrc configuration file and must be an NPM package of the name (@[^/]*)?/parcel-resolver-*. This becomes very difficult since I'm trying to use a locally defined plugin, not one which exists on NPM. Parcel does support local plugins, but they need to be loaded via NPM workspaces or Yarn link:, neither of which are easy to do within a Bazel context.

I eventually got Parcel to accept a provided .parcelrc file but it was struggling to import the resolver. I eventually got this working with npm_install(links = {"parcel-resolver-bazel": "//path/to/resolver"}) which makes an alias for @npm//parcel-resolver-bazel and sets up a node_modules/parcel-resolver-bazel directory with the contents of the given target. This was enough for Parcel to import the resolver. Unfortunately it is still quite limiting because it can't easily import other JS modules in the workspace (that would import from a node_module out to the main repository, which is a backwards dependency) and vendoring dependencies in the node_module directory becomes quite complicated. I was able to workaround this by sharing data with globals, but it is fairly limited and could be an issue in the future.

One other challenge I haven't fully solved yet is how to ship this plugin approach to downstream users. @npm//parcel-resolver-bazel won't exist in user workspaces because that's not a real NPM package. Some immediate ideas of how to solve this are:

  1. Document that users must add npm_install(links = {"parcel-resolver-bazel": "@npm//rules_prerender:parcel-resolver-bazel"}). This is awkward, easy to forget, and forces users to use NPM even if they don't want to do that (admittedly this is a requirement today for rules_prerender, but something I'm hoping to move away from eventually).
  2. Have rules_prerender workspace dependencies create our own npm_install(name = "npm_rules_prerender_internal") with an empty package.json and the single link for parcel-resolver-bazel, then always use that for the plugin. Not sure if this would cause conflicts with the @npm workspace when used in the same target.
  3. Actually publish parcel-resolver-bazel to NPM and depend on it in the rules_prerender NPM package or make users add the explicit dependency. This creates a pretty significant divergence between the way it works locally in rules_prerender and the way users actually consume it via NPM, but we already have some significant divergence here, so maybe that's just ok?

Depending on how hard getting this to work in external workspaces is, I'm starting to wonder if it would be easier to fork rules_postcss than to try and leverage Parcel in this manner.

I'll also follow up with Parcel folks to confirm that I really do have to go through all this NPM nonsense. It's a lot of ceremony to deal with in order to import a plugin, and there should definitely be a simpler way to do this.

dgp1130 commented 2 years ago

Snapshot of the current state is in ref/parcel-2.

dgp1130 commented 2 years ago

One other note: I had to abuse a global variable in order to have the CSS import map loaded into the Parcel binary get passed through the resolver plugin. Since Parcel manages loading and creating the plugin, there is no way to pass through use case specific context, and this was the best workaround I could find.

dgp1130 commented 2 years ago

Latest suggestion is to implement the resolver in Rust since that API is actually supported. I hacked on it a bit and managed to get something working in ref/parcel-3, the Bazel side is infinitely simpler this way. That said I do have a few concerns:

  1. The parcel_css crate is still in alpha (version 1.0.0-alpha.24 as of today). I would rather not depend on prerelease code if I can avoid it, but also feel like 80% of the Rust crates I see haven't yet hit 1.0.0 despite being critical to the ecosystem.
  2. The resolver only see the full file path after Parcel has already joined it with the path of the originating file. That means that an absolute import of rules_prerender/foo.css from rules_prerender/path/to/bar.css results in a file resolution request for rules_prerender/path/to/rules_prerender/foo.css, which definitely doesn't exist. I think the problem here is that Parcel is assuming these specifiers work like typical file paths, when they technically don't in Bazel (${workspace_name}/... is treated as an absolute path). I could request a change in Parcel to get the originating file and the import specifier or accept that absolute imports aren't possible with typical Bazel syntax.
  3. Using the Rust toolchain means a lot more infrastructure I have to build out, as well as managing a new set of dependencies. In particular, I will need to figure out how to ship the precompiled Rust binary to users. That might involve compiling to WASM and calling it from JS, shipped to the rules_prerender NPM package, or I could explore bzlmod. Either way, there's a lot of open questions to figure out there.

This definitely feels better than trying to use the Parcel API, but there's a lot of new Rust-based problems I'm not familiar with which will take some solving before this becomes a viable approach.

dgp1130 commented 2 years ago

Since rules_postcss is locking this repository to Node v12 which is already unsupported, this is a hard blocker for a 1.0.0 release.

dgp1130 commented 2 years ago

I just realized that the actual @rules_postcss issue only occurs when generating sourcemaps, however this never really worked in @rules_prerender anyways because it wasn't served correctly and I never bothered to get around to fixing it. Sourcemaps are a much smaller feature, particularly related to CSS files, so I think it's worth turning that off to unblock the Node.js upgrade. We can revisit CSS sourcemaps after to the move to Parcel. Done in 39a1fb6ed30c966a82a267f2b655f35ef7e8823b and aafa0608cc56e91d11625668c3ebd6104fcdfb5c.

dgp1130 commented 1 year ago

In #48, we've now moved to @aspect_rules_js and should have an easier time supporting Parcel. This is the last major piece of infra which still requires @build_bazel_rules_nodejs.

I took a pass at this today and was able to make it work relatively easily. I'm not a fan of the dependency layout. parcel (the CLI) contains a direct dependency on @parcel/config-default (the default configuration). That might make some sense, except that the default config is basically an "everything config". It includes plugins for CSS, JS, GraphQL, Stylus, Yaml, etc. (source). This also means it depends on all those plugins. We only actually want it for CSS, nothing more. I made my own config which only included the CSS relevant plugins, however we still install all the plugins used by the default config, even though the default config isn't actually used.

After figuring out the dependencies I was able to make it work without too much effort. I got it working in an external workspace and was even able to remove all the PostCSS stuff, though Parcel is using PostCSS under the hood. I haven't bothered with strict deps yet, and it might be out of scope for this issue, but that would definitely be good to add sooner than later.

dgp1130 commented 1 year ago

Before I refactor and merge the branch using the parcel CLI directly, I do want to try using @parcel/css directly, since I think that might be the better level of abstraction to use for this problem. Apparently @parcel/css has been rebranded as lightningcss, but seems to be mostly the same thing.

After spending some time with that, I was able to make a css_bundler tool which calls lightningcss under the hood to bundle a number of entry points. This has significantly fewer dependencies and everything is actually appropriately used, so I think this is definitely the right path forward. Based on this I was able to create a new css_bundle() rule, simplify the CSS import map and delete all the PostCSS usage.