Add transformTemplate() function to vite plugin #30
Conversation
rreusser
changed the title
|
Although for the sake of argument, what if For comparison, there’s the I’m open to anything that gets the job done though! |
There was a problem hiding this comment.
Thanks for the PR. This seems useful!
This API feels pretty good to me. I don’t have too many qualms about exposing this API, even if it is perhaps more than necessary.
I don’t think we want to make template a (polymorphic) function. While that sounds attractive, the plugin should be able to watch the template file so that it can trigger a reload when the template is edited; if we abstract the template as a function rather than a file, the plugin won’t be able to watch the file, so the one-liner equivalent using fs.readFile isn’t really equivalent.
My only thought is to change the signature of transformTemplate so that the template source (the UTF-8 decoded contents of template file) is the first argument, and options: {input, context, notebook} is the second argument. Or maybe we pass context: {...context, input, notebook} as the second argument? That feels right. Though I don’t like the name input since it is ambiguous; it’s the source of the notebook HTML file, not the template. Do you think it’s necessary to provide both the parsed/deserialized notebook and the original HTML source of the notebook? In any case, it would be nice if the default implementation of the template transform can be the identity function (source) => source.
Also, we should await the result of transformTemplate so that you can have an async transform if desired.
I guess it would be okay, too, if we fold the template source into the options argument… but I think it should be called source rather than template since we’re already using the template name to refer to the path to the template rather than its contents. (And you’re calling it tsource internally here… which feels like an indication we should call it source in the transform function signature.)
|
Could also have a separate |
Good call. Agreed.
Agreed, I don't think it's necessary. No sense going overboard eschewing convenience, but……… what if it only received the HMR context containing primarily just the path/filename? It only takes an import/instantiation and a one-liner to recover the HTML source and parsed notebook, and this way there's no risk of incidental mutations (unless that were enabled with a dedicated I don't think this is too much to ask if you want the parsed notebook: import { JSDOM } from 'jsdom';
const window = new JSDOM().window;
const parser = new window.DOMParser();
...
observable({
template: TEMPLATE_PATH,
transformTemplate: async function (template, {path, filename}) {
const notebook = deserialize(await readFile(filename, 'utf8'), {parser});
...
return template;
})
})
|
|
I've cleaned this function up according to the feedback. Edit: I've also passed through vite server, rollup bundle, and rollup chunk from the plugin context. I don't exactly know the use, but perhaps no sense filtering it. See ctx parameter of |
Co-authored-by: Mike Bostock <[email protected]>
Co-authored-by: Mike Bostock <[email protected]>
|
I've made the suggested changes. Good call, it feels cleaner now. Thank you! |
Desired behavior
I would like to insert information obtained from either the notebook or from the notebook filename into the page template. This includes, for example, inserting a published-at date into the page, inserting meta tags with a description/title/image into the
<head>element, or using a page-dependent identifier to append a comments box.Actual behavior
The
templateargument of theobservable()vite function only accepts a filename, making it difficult to customize per-page.Proposed solution
Rather than making
templatepolymorphic and optionally accepting a function which returns the template string, I've added atransformTemplate()function which receives… probably too much metadata about the notebook. This allows me to write my template as a Handlebars.js template and use thetransformTemplate()function to compile the template, inserting page-dependent content into the template. (Full disclosure: I also mutated the parsed notebook to inject some extra content into the header cell.) The page-dependent content is obtained by tracing the notebook filename to ametadata.ymlfile in the corresponding directory.See: https://github.com/rreusser/notebooks/blob/c1b9efd5d5a3a21f24b9a96e2ac44eb7ec83bccc/vite.config.js#L25-L60
Concerns
At the end of the day, this PR is a shot in the dark since as a habit, I try to propose fixes rather than just requesting features. I won't take it hard if this isn't an acceptable approach though. I'm glad to follow through and add tests/docs, or just bail on this and take a different approach entirely. Notebooks 2.0 is outstanding so far.