search

dgp1130 / rules_prerender

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

Refactor repository to use the `data` attribute on `ts_library()` #31

Closed dgp1130 closed 1 year ago

dgp1130 commented 3 years ago

I recently learned that ts_library() supports a data attribute, which I previously did not think existed for reasons I don't entirely remember. The data field was also recently added to ts_project(), though I don't believe that has released yet. Throughout the repository, there are a number of places where data dependencies are shifted to a nodejs_binary() or jasmine_node_test() rule, when they should be applied to the ts_library() which actually uses the resource.

I started to look into this, but ran into issues with DefinitelyTyped packages. Apparently having a dep on @npm//yargs with a data dep on @npm//@types/yargs does not work as expected. I believe some transitive runfiles are not being included, but I need to do some more research. This is likely a ts_library() runfiles bug, but once we are able to resolve that, we should update our code to keep data dependencies as close to their usage as possible.

dgp1130 commented 3 years ago

I did some more investigation into the underlying rules_nodejs issue but wasn't able to pinpoint a fix. The issue doesn't apply to all packages, but in this repository it mainly affects yargs and puppeteer, which are the primary use cases for this refactoring.

For now, we're just blocked on https://github.com/bazelbuild/rules_nodejs/issues/2489.

dgp1130 commented 1 year ago

We've migrated to ts_project() from @aspect_rules_js, so this issue is obsolete.