dgp1130 / rules_prerender

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

Relative `includeScript()` and `inlineStyle()` imports #69

Closed dgp1130 closed 1 year ago

dgp1130 commented 1 year ago

Currently includeScript() and inlineStyle() require full workspace-relative paths:

includeScript('path/to/pkg/my_script')
inlineStyle('path/to/pkg/my_style.css')

This is annoying and verbose, ideally we should be able to do relative paths given that 99% of the time a component's prerender logic is in the same directory as its script or style. This is also more aligned with JavaScript and @aspect_rules_js which tend to use relative paths for these kinds of things (the above paths would be interpreted as belonging to the path package in a strict Node sense).

The two paths forward here are either:

import includeMyScript from './my_script' assert { type: 'prerender-script' };
import includeMyStyle from './my_style' assert { type: 'prerender-style' };

This might be possible to do with a loader though that's still experimental. It would likely also require a TypeScript plugin so it understand the type of the result from the assertion.

The other path is to use import.meta.url like so:

includeScript('./my_script', import.meta.url);
inlineStyle('./my_style', import.meta.url);

The given path can be evaluated relative to the URL of the source file. This seems like the most straightforward approach and the easiest to implement, so we should probably start there.

dgp1130 commented 1 year ago

One downside to the second approach is that it isn't compatible with strict dependencies since it isn't evaluated until runtime when all transitive components have their scripts and styles available. The first approach is actually checked at build time with only resources known to the component. So we could perform strict deps checking on that approach, but not the second.

dgp1130 commented 1 year ago

I was able to update to use the includeScript('./foo.mjs', import.meta) pattern without too much effort. This introduces a few new constraints on user code:

  1. Imports must be relative paths (start with ./ or ../).
    • This aligns with ESM / Node import semantics (relative paths are files, bare imports are packages).
    • Eventually we may want to support bare imports for including third party code, but for now that is unsupported.
  2. Imports must include file extensions (end with .js, .cjs, .mjs, or .css).
    • This aligns with native ESM semantics.
    • In the future, we could take the package.json file into account, but there's not much need for this when loading internal scripts or styles.
  3. Including scripts or styles from external repositories is not supported.
    • It was already very poorly defined behavior and I'm not even sure if it worked.
    • In the future, bare imports of NPM packages are likely the better approach to including third party code.

Ultimately the syntax is much nicer to work with and I'm pretty happy with it. It still doesn't solve the strict deps problem, but we have #2 and #3 to look into that.