Add support for custom scripts

[Lucca-mito]

Summary

First-class support for adding custom scripts to a DocC-generated website. These may be local scripts, in which case the website will continue to work offline, or they may be external scripts at a user-specified URL. This support is in the form of a custom-scripts.json file, the scripting analog of theme-settings.json.

Full pitch: https://forums.swift.org/t/pitch-support-for-custom-scripts-in-docc/75357.

Dependencies

Corresponding change in swift-docc.

Testing

• Unit tests added.
• To test the feature end-to-end, you'll need to use the version of swift-docc with the corresponding changes. Follow the steps in the pitch for adding any custom scripts you want to your documentation website (your own scripts, or a library such as MathJax or D2). If you want a specific example to test the implementation:
  • Copy the custom-scripts.json and the custom mathjax-config.js script (shown in the pitch) to your documentation catalog.
  • Add LaTeX expressions to your in-source documentation.
  • docc convert the documentation catalog with your custom scripts. Observe that the modified swift-docc copied custom-scripts.json and the custom scripts into the documentation archive.
  • Preview the documentation archive. Observe that the LaTeX expressions are rendered on the page.
• Live demo: see here and here. The LaTeX expressions in the documentation text are dynamically rendered into equations via custom scripts.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

• Added tests
• Ran npm test, and it succeeded
• Updated documentation if necessary
  • Existing documentation does not need to change. New documentation for this feature will be added after the design and implementation are finalized.

[mportiz08]

Very cool to see this in action, and I know this kind of customization will be super useful to some developers. Nice work!

I'm very interested to hear other feedback from the community on your forums pitch.

Here's some initial feedback on the renderer implementation itself in the meantime.

[mportiz08]

I'm doing a quick test with a very simple script that looks like this:

[
  {
    "code": "window.alert(\"hello world\");",
    "run": "on-load-and-navigate"
  }
]

My expectation is that the popup happens once on the initial page load (1) and also after the page loads for every link I click (2).

What actually happens for scenario 2 is that I get the popup immediately after clicking a link when the URL has changed to the new page, but the HTML is still showing the previous page I was navigating away from.

Question: Is my expectation wrong or is this just possibly an issue with the implementation?

If it's an issue with the implementation, we would probably want to look into using the vue-router hooks and guards for calling these functions instead of the $route watcher that you have in the root component here.

[Lucca-mito]

The visual issue you’re describing in scenario 2 is a quirk of alerts: despite what the browser may be displaying, the alert in scenario 2 is fired strictly after the link is clicked and the subpage is loaded. To prove this, try the following script instead:

window.alert(document.querySelector('h1').innerText);

and notice that the alert text matches the H1 of the subpage to which you’re navigating, not of the subpage you’re in, regardless of what the browser is displaying. This means that, in scenario 2, the script is running strictly after navigation.

As an additional test, also try:

document.querySelector('h1').style.fontStyle = 'italic';

and notice, in scenario 2, that the H1 (dynamically inserted into the subpage to which you're navigating) is successfully styled, which means that the script did run strictly after navigation.

As for using vue-router hooks instead of the $route watcher, I agree that that would be the more correct approach. I’ll work on it.

[mportiz08]

The visual issue you’re describing in scenario 2 is a quirk of alerts: despite what the browser may be displaying, the alert in scenario 2 is fired strictly after the link is clicked and the subpage is loaded. To prove this, try the following script instead:

window.alert(document.querySelector('h1').innerText);

and notice that the alert text matches the H1 of the subpage to which you’re navigating, not of the subpage you’re in, regardless of what the browser is displaying. This means that, in scenario 2, the script is running strictly after navigation.

As an additional test, also try:

document.querySelector('h1').style.fontStyle = 'italic';

and notice, in scenario 2, that the H1 (dynamically inserted into the subpage to which you're navigating) is successfully styled, which means that the script did run strictly after navigation.

Whoops. Got it, that makes sense.

As for using vue-router hooks instead of the $route watcher, I agree that that would be the more correct approach. I’ll work on it.

Sounds good. I think this approach is fine after realizing my problem was with my example, although it still might be good to use those hooks if possible to be safe from any potential issues related to the navigation cycle.

[mportiz08]

Is the reason why you're using eval instead of import just for the inline code use case?

Unless there's a strong reason for it, I would suggest maybe eliminating the inline code option and using import instead, especially since all inline code could easily be extracted into a simple local script.

[Lucca-mito]

I’m using eval instead of import just to support inline scripts; though, for sandboxing and performance, I should’ve used new Function instead. Please let me know whether I should:

• Replace eval with new Function and keep inline scripts.
• Replace eval with dynamic imports and remove inline scripts.

[mportiz08]

I would personally prefer the latter since all inline scripts could be expressed as simple local scripts, and I would much prefer using dynamic imports over manually evaluating the code. I could be convinced otherwise if there's a strong need to explicitly support inline code, but I think the dynamic import approach would be simpler code-wise and also less prone to possible issues.

[mportiz08]

Would this maybe make sense to live in the src/utils/data.js file where we have similar functions for fetching data instead of creating a new file just for this?

(Not a major issue, and there's a possibility this function might not even be needed based on my other comment about avoiding eval if possible.)

[Lucca-mito]

Sounds good, I’ll move fetchText to src/utils/data.js — unless we switch to dynamic imports, in which case I’ll remove it.

[mportiz08]

Is there a reason why the linter needed to be disabled for this entire file or was it just for the one line where you disabled it inline? Just curious.

[Lucca-mito]

That’s a mistake! A relic from when I littered the file with console.log statements for debugging. Thanks for catching it. 😮‍💨

[mportiz08]

One existing problem we have with the same pattern from theme-settings.json is that we always try to fetch the file, and the 404 is always reported in the console in the case where one wasn't provided.

I don't think this needs to be solved with this PR, but it would be nice to fix this for both files so that the renderer is informed in some way of the fact that the catalog even has one of these files before it tries to fetch them, so this "always 404" problem goes away in the normal case.

(This would need to be addressed in a coordinated way between DocC and DocC-Render in the future)

[Lucca-mito]

Yes, that's an unfortunate behavior of fetch... something we can look into in a later PR.

Sidenote: I should probably point out that the way I’m error-handling custom-scripts.json intentionally does not match how theme-settings.json is error-handled. fetchThemeSettings has a blanket catch statement, which means that documentation authors aren’t notified if the file is malformed or if it violates the theme-settings schema. Messing with this choice for theme-settings.json is outside the scope of the proposal, so I didn’t; but for custom-scripts.json, if the file is malformed or violates the custom-scripts schema then we throw an error and don’t catch it. Please let me know if schema violations should be caught for custom-scripts.

[mportiz08]

That sounds good.

I don't think it's necessarily a problem with the fetch behavior—it's just that we shouldn't even be trying to fetch any of these JSON files if they haven't been provided in the documentation catalog in the first place—we would just need some added infrastructure so that DocC could inform the renderer that it will never even need to make an http request for these files if they weren't used.

Again, not any issue with your implementation—just noting an existing problem that will happen here as well.

[mportiz08]

Very minor thing, but it would be great to have some constants or an enum-like Object for some of these strings so that we don't have to rely on them always being typed exactly the same in all the places in the code. (Wish we were using typescript so this kind of thing was a little more straightforward)

Something like:

const Run = {
  onLoad: 'on-load',
  onLoadAndNavigate: 'on-load-and-navigate',
};

or

const RunOnLoad = 'on-load';
const RunOnLoadAndNavigate = 'on-load-and-navigate';

etc

[Lucca-mito]

Will do!