dgp1130 / rules_prerender

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

Preact templating engine #71

Closed dgp1130 closed 1 year ago

dgp1130 commented 1 year ago

With the Lit templating engine currently stalled in https://github.com/dgp1130/rules_prerender/issues/70, one possible alternative is Preact. I'm not very familiar with the React / Preact ecosystem, but I think it should be possible to do this.

Apparently Preact includes semi-official support of the htm package which does not use JSX and even looks a lot like lit-html.

I was able to make an initial prototype which mostly works. Got a bit confused as to why <!DOCTYPE html> and <meta charset="utf8"> render so weirdly, but eventually figured out workarounds.

I then tried to include a script, only to discover that Preact (JSX? VDom?) cannot represent comments as there's no way to render them. This is a problem for includeScript() and inlineStyle() which render comments as annotations later processed by the build system.

I'm not immediately able to find a workaround, but I'll have to keep looking. One possible approach would be to change annotations to output actual HTML instead of comments like:

<rules_prerender:include-script path="path/to/pkg/script.js />

This might be useful as I ran into a similar problem when experimenting with SSR, which required nested annotations. Nesting HTML comments is actually kind of a pain in the ass since they are parsed lexically, not grammatically. Using real HTML could solve both problems.

dgp1130 commented 1 year ago

developit actually responded to some of my complaints around Preact and confirmed that there is no good way to render comments: https://techhub.social/@develwithoutacause/110003437970303438.

I started looking into using <rules_prerender:annotation>{...}</rules_prerender:annotation> and this does appear to be doable. Unfortunately, it will get printed to any .textContent usage, which leaks into user tests. Instead, I tried <template rules_prerender_type="annotation">{...}</template>, however this still surfaces in .textContent because node-html-parser doesn't special case templates like it should. I filed https://github.com/taoqf/node-html-parser/issues/235 to follow up with a proper fix for that. For now, I'll just need to update those tests to be less brittle, but it is unfortunate that these annotations leak into user code like that.

Jason also suggested that the lack of validation in htm is somewhat intentional. JSX is intended to provide a proper compile step with validation, while htm isn't really intended for that use case. Validation could certainly be improved, but it's not as necessary for htm when JSX already provides that as an alternative. Given that JSX is directly supported in TypeScript, I think it's probably better to use that for @rules_prerender examples, though we should be flexible enough for users to plug in their own VDom renderer. I'm not sure if that's the right terminology, but basically users should be able to choose whether or not to use JSX, htm, or anything else for their templates, as long as it is compatible with Preact.

Using <rules_prerender:annotation />, I was able to make an includeScript() and inlineStyle() work. Unfortunately <template /> doesn't work out of the box, but I was able to add support for it via a relatively straightforward <Template /> component.

I tried to move off htm and instead use TSX directly. Unfortunately .mtsx doesn't appear to be a valid extension. We can use *.tsx and it compiles successfully, but outputs a *.js file instead of a *.mjs file. Surprisingly this actually worked, however that is only because it appears to be incorrectly reading the root package.json file outside the sandbox and using it's "type": "module" configuration. Technically I need an explicit data dependency on the root package.json (copied to bin ofc) in order to build this correctly. It's not a terrible workaround, but definitely unintuitive and shouldn't be necessary for every prerender_pages() target.

Regardless I think this is a good enough state to ship in, mostly works with the only major caveat being the file extension issue.

dgp1130 commented 1 year ago

One problem I haven't solved yet is using @rules_prerender/declarative_shadow_dom. polyfillDeclarativeShadowDom() currently returns a string, but AFAICT there's no easy way to parse an HTML string into Preact VDom without another dependency. I'm thinking this could be solved with a import { polyfillDeclarativeShadowDom } from '@rules_prerender/declarative_shadow_dom/preact'; which uses an optional peer dep on @rules_prerender/preact. I'll have to play around with it.

dgp1130 commented 1 year ago

So I tried updating an out-of-repo example I have lying around and ran into an unexpected problem. I can't get TypeScript to type check a custom element in JSX. Apparently you're supposed to extend the JSX.IntrinsicElements interface much like HTMLElementsTagNameMap. But no matter what I try I can't get it to work:

declare global {
    namespace JSX {
        interface IntrinsicElements {
            'my-component': any; // Doesn't have any effect.
        }
    }
}

export function Component({ children }: { children: ComponentChildren }): VNode {
    // Error: Property 'my-component' does not exist on type 'JSX.IntrinsicElements'.
    return <my-component></my-component>;
}

The best workaround I can find is to use createElement('my-component', {}, [ /* ... */), but that definitely shouldn't be necessary. I have no idea what I'm doing wrong here, but I'll have to keep investigating.

dgp1130 commented 1 year ago

So apparently the correct syntax is:

declare module 'preact' {
    namespace JSX {
        interface IntrinsicElements {
            'my-component': {};
        }
    }
}

It feels very hacky to merge into a namespace of a dependency like this, though I'm not sure declare global {} is all that much better, so đŸ¤·.

Playing around with this type, it seems kind of annoying for a lot of components, but also pretty useful for type checking prerender output. If you really want to turn it off, you can do:

declare module 'preact' {
    namespace JSX {
        interface IntrinsicElements {
            [comp: string]: {[prop: string]: unknown};
        }
    }
}

This still supports overriding specific elements with more precise types, so it's actually pretty nice for making things more ergonomic IMHO, though at the cost of strictness. You could easily typo my-compenont and be confused why things don't work at runtime. In fact, in my prototypes, the #1 mistake I make is mistyping the tag name in some capacity, which is annoying to debug because it has no effect on the page, the web component just doesn't execute and no error is surfaced. (Maybe we could fix that with a DevTools extension - https://techhub.social/@develwithoutacause/110012548273306004.)

We can give this a more specific type, but it's a little annoying since the actual "definition" is in the client code, not the prerendering code. Usage of this.getAttribute() implies some type definition on the component's attributes (maybe HydroActive could solve that - filed https://github.com/dgp1130/HydroActive/issues/1 to follow up on it). Unfortunately there's no clear way to emit that as an inferred type. The component itself can define an interface, though it shouldn't depend on preact which is annoying here.

I updated the Template type to properly pass through standard HTML properties. I briefly considered defining a new intrinsic for <template /> in @rules_prerender/preact, but decided against it because that would modify the global types and could be confusing in a Bazel context where not all ts_project() targets will depend on @rules_prerender/preact, so it's only kind of global in an awkward way.

dgp1130 commented 1 year ago

I'm trying to look into updating @rules_prerender/declarative_shadow_dom to expose a /preact path, but this seems to require moduleResolution: "Node16" in TypeScript, which introduces a bunch of module resolution challenges I still don't understand. I wasn't able to find any existing @aspect_rules_ts example which uses Node16, so I'm not sure if this possible or what the problem even is.

Instead I just exposed this functionality in @rules_prerender/declarative_shadow_dom/preact.mjs. There's nothing restricted access to package internals, which I don't like, but this is good enough to solve the problem. I should look into proper exports support in the future.

dgp1130 commented 1 year ago

Released everything in 0.0.26. Next steps are:

  1. Migrate all the internal examples to use Preact (because I want to strongly discourage direct string manipulation).
  2. Add TextResource type which PrerenderResource allows as a string input without sanitization.
  3. Remove string inputs from PrerenderResource in favor of SafeHtml and TextResource.
  4. Remove / discourage any raw string manipulation-based APIs (such as the built-in includeScript() and inlineStyle() as well as the default polyfillDeclarativeShadowDom()).

I don't think those need to be in this issue, so I'll follow up separately with those.