Currently, the includeStyle() function does not validate that the included file is present in the styles attribute of the associated prerender_component() rule. If you include a style which you don't depend on, it could throw a file not found error at the bundling step. Worse, if another component depends on that file, it could pass the build until that component changes in the future and removes the style. Simply put, prerender_component() should verify that all includeStyle() expressions have files which are direct dependencies of the styles attribute.
How to do this could be quite tricky. Since we need to throw an error in a ts_library() component, the only want to do that is to make this state a TypeScript compile error. I can think of two ways to do that:
Add a plugin to check strict deps. This is how ts_library() solves the problem today and we could do something similar. The downside here is that we need to somehow add this plugin. Users may have their own version of ts_library() which applies some of their own plugins, and we need to be compatible with that. Considering that right now we don't allow custom ts_library() implementations, this may not be too bad, but it is something we will have to deal with.
Generate the includeStyle() implementation with a more specific type. Instead of depending on @npm//rules_prerender to provide the includeStyle() function, we could instead have prerender_component() read the styles attribute and generate a ts_library() which provides includeStyle(). This generated version could then accept as input a union of all the valid strict deps, rather than a simple string. This dodges the plugin problem but adds a lot of its own complexity.
We also have to consider what happens if a user calls includeStyle() with a variable that can't be resolved to a specific string. There shouldn't be much use for that, but I'm sure users will try it. This should probably be an error in order to improve strictness, but we may need some means of ignoring such cases.
There is also the question of how we distinguish direct dependencies vs transitive dependencies. In ts_library(), there is a srcs and a deps attribute to distinguish the two, but CSS files use the filegroup() rule as library abstraction. That only has a single srcs attribute, so it may not be possible eliminate transitive deps. Will need more investigation here.
Currently, the
includeStyle()function does not validate that the included file is present in thestylesattribute of the associatedprerender_component()rule. If you include a style which you don't depend on, it could throw a file not found error at the bundling step. Worse, if another component depends on that file, it could pass the build until that component changes in the future and removes the style. Simply put,prerender_component()should verify that allincludeStyle()expressions have files which are direct dependencies of thestylesattribute.How to do this could be quite tricky. Since we need to throw an error in a
ts_library()component, the only want to do that is to make this state a TypeScript compile error. I can think of two ways to do that:ts_library()solves the problem today and we could do something similar. The downside here is that we need to somehow add this plugin. Users may have their own version ofts_library()which applies some of their own plugins, and we need to be compatible with that. Considering that right now we don't allow customts_library()implementations, this may not be too bad, but it is something we will have to deal with.includeStyle()implementation with a more specific type. Instead of depending on@npm//rules_prerenderto provide theincludeStyle()function, we could instead haveprerender_component()read thestylesattribute and generate ats_library()which providesincludeStyle(). This generated version could then accept as input a union of all the valid strict deps, rather than a simple string. This dodges the plugin problem but adds a lot of its own complexity.We also have to consider what happens if a user calls
includeStyle()with a variable that can't be resolved to a specific string. There shouldn't be much use for that, but I'm sure users will try it. This should probably be an error in order to improve strictness, but we may need some means of ignoring such cases.There is also the question of how we distinguish direct dependencies vs transitive dependencies. In
ts_library(), there is asrcsand adepsattribute to distinguish the two, but CSS files use thefilegroup()rule as library abstraction. That only has a singlesrcsattribute, so it may not be possible eliminate transitive deps. Will need more investigation here.