search

dgp1130 / rules_prerender

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

Remove non-multi versions of tools #25

Closed dgp1130 closed 3 years ago

dgp1130 commented 3 years ago

As of now there are a few tools with multiple versions, a basic version which processes a single HTML file, and then a "multi" version with processes multiple. I'm curious if I can drop the basic version and update the Starlark rules to use the "multi" versions with just a single input. This is a bit awkward for a few reasons, but it may be worth doing if we can drop a lot of the extra complexity.

dgp1130 commented 3 years ago

I was able to pretty easily delete prerender_page_bundled() as it is just a subcase of prerender_multi_page_bundled(). The only feature it provided was the path argument, which I don't think is very useful. The easier answer is to just tell people to use yield PrerenderResource.of('/my/path.html', /* ... */), rather than using prerender_page_bundled(path = "/my/path.html").

I updated all internal usages to use prerender_multi_page_bundled(), however I think we can probably simplify the name. I'm thinking we could change it to something like prerender_resources(), prerender_pages(), prerender_bundle(), prerender_site(), etc. Still need to workshop the final name.

Next step is to try to simplify prerender_page() to use prerender_multi_page() under the hood. This will be a little more complicated because we can't just delete prerender_page() and it does have a valid use case for returning a direct string rather than an Iterable<PrerenderResource>.

dgp1130 commented 3 years ago

At this point, we have successfully removed prerender_page() and the original renderer, having renamed the multi-renderer to "renderer". Next steps are to remove the original annotation extractor and the original resource injector.

dgp1130 commented 3 years ago

I should also update entry_point.ts to no longer accept a string result, because that is no longer used now that we dropped the original renderer.

dgp1130 commented 3 years ago

I've removed the original annotation extractor and resource injector and renamed multi-annotation extractor and multi-resource injector to drop the "multi" prefix that is no longer required.

I also updated entry_point to no longer accept a string or Promise<string> from user code, since there is no build rule which will accept that anymore.

I think all the significant cleanup is done and the repository is looking a lot better already.