saneef / eleventy-plugin-asciidoc

Eleventy plugin to add support for AsciiDoc.
MIT License
24 stars 5 forks source link

Impossible to register asciidoctor extensions #10

Closed tbroyer closed 11 months ago

tbroyer commented 12 months ago

With version 2.0.0 of the plugin, it was possible to register asciidoctor extensions using code like:

const eleventyAsciidoc = require("eleventy-plugin-asciidoc");

const asciidoctor = require("asciidoctor").default();
const registry = asciidoctor.Extensions.create();
require("./my-extension.js")(registry);

module.exports = function (eleventyConfig) {
  eleventyConfig.addPlugin(eleventyAsciidoc, {
    extension_registry: registry,
  });
};

This no longer works with version 3.

I suppose it comes from upstream Asciidoctor.js changes that now requires the registry to have been created from the same asciidoctor instance; anyway, we cannot update to version 3 due to that change, and I'd suggest providing a proper and future-proof way to register asciidoctor extensions in the plugin: a callback function passed in the options that the plugin would call (if present) with an extension registry (or with the asciidoctor.Extensions object directly).

From the user point of view, it would be used as:

const eleventyAsciidoc = require("eleventy-plugin-asciidoc");

module.exports = function (eleventyConfig) {
  eleventyConfig.addPlugin(eleventyAsciidoc, {
    register_extensions(registry) {
      require("./my-extension.js")(registry);
    },
  });
};

And from inside the plugin, it could be along the lines of:

const { register_extensions, ...options } = eleventyOptions;

if (register_extensions) {
  const registry = processor.Extensions.create();
  register_extensions(registry);
  options.extension_registry = registry;
}
saneef commented 12 months ago

Thank you, @tbroyer, for reporting. I'll look into it.

saneef commented 12 months ago

@tbroyer To me, it looks like a problem with the latest asciidoctor.js. Our existing way of registering extension works with the first call of convert(), but not with the subsequent calls. 😞 See my comments on asciidoctor/asciidoctor.js#1709

Let's wait for their reply.

tbroyer commented 11 months ago

Fwiw, from a related issue: https://github.com/ggrossetie/asciidoctor-kroki/issues/421#issuecomment-1649374749

A registry is not really designed to be reused. Ideally, it should be created per conversion.

It looks like providing a function in the options is not that bad an idea actually. IIUC, you could either give it once to asciidoctor.Extensions.register(), or call it once per conversion each time with a new registry.

saneef commented 11 months ago

Yup! I saw that comment. TBH, I was hoping there might be some fix from their side. 😄 I have seen our current approach being used in other projects. Now many of those need to be changed to re-create the registry for each conversion.

Having said that, I'll work on the approach, like you have suggested, using a function which gets a new registry for each conversion. I had tried asciidoctor.Extensions.register(), but it didn't work for me.

Will make a new release in a couple of days.

saneef commented 11 months ago

I have published a new release, v3.1.0.