Pass handler list to overriden handlers

[stevemk14ebr]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

Remark stringify currently accepts a list of handler overrides for how to stringify a node:

RemarkStringify, {handlers: {
    table: (node, parent, context) => {
        ...code...
    }
}

This table acts as a full override, however, it's sometimes useful to use the original behavior but apply some post processing. An example is for tables, where RemarkGFM does quite some work to serialize to string, I have code that just need to apply a particular regex to that output, but this is currently not possible.

Solution

Provide an additional argument to the handler callbacks so that they can observe the handler array BEFORE user applied applied overrides. Example desired interface:

RemarkStringify, {handlers: {
    table: (node, parent, context) => {
        let tmp = context.**originalHandlers**.table(node, parent, context);
        ...code modified tmp...
        return tmp;
    }
}

Alternatives

Change the RemarkStringify interface to have a new type of callback that is invoked after each stringify handlers runs. This new handler would be passed the string value output where it may transform it, then return a final value.

ex:

RemarkStringify, {posthandlers: {
    table: (txt) => {
        ...code modified txt...
        return txt;
    }
}

[wooorm]

What do you think of instead, in index.js, re-exporting all of these?
You can then depend on this package yourself, version with semver, while tree-shaking will only pick out the one thing (tables) that you need?

[stevemk14ebr]

That wouldn't handle extensions like GFM (table, rows, etc) though right? If i re-exported those handlers then only the base ones would be there? Or are you suggesting something else, sorry I don't follow?

I care about the handlers from these in particular:

mdast-util-to-markdown/lib/configure.js

Line 19 in d51bf74

configure(base, extension.extensions[index])

[wooorm]

Good catch, I missed that!

We could do the same for those packages.

Especially for GFM features, originalHandlers feels off: as GFM handlers aren’t “original” at all?

[stevemk14ebr]

That would work, but the downside to that is that there's lots of nested exports to export due to the structure of the GFM stuff.

To avoid that the handlers passed into options by users could be renamed 'handlerOverrides' and then this library could prefer to dispatch overridden handlers vs the original 'handlers'. Due to how the data binding works the original handlers are already available via the context parameter.

[wooorm]

Does your code actually do that? I haven’t tested with this package and the GFM packages, but it looks like the actual handlers are the same as “originalHandlers”, so that if you overwrite table with function x, then context.originalHandlers.table will be x?

[stevemk14ebr]

My PR will make it so that all of the handlers registerd by extensions & default list are put on the key 'originalHandlers'. The user overrides are not put into this list due to this line:

mdast-util-to-markdown/lib/index.js

Line 35 in fa06b79

configure(context, options)

Above I'm suggesting a change to this logic. Instead of making an originalHandlers key, the line I highlighted above would be changed to something like this:

let tmp = {...options};
tmp.handlerOverrides = tmp.handlers;
delete tmp.handlers;
configure(context, tmp)

Then during dispatch the library would internally prefer handlerOverrides.

Up to you what's preferred.

[wooorm]

What you‘re trying to achieve seems to be get a “previous” handler: One extension defines a table handler A, another defines a table handler B. B wants to get access to A.

This is interesting, but what if:

• There’s another extension that adds table handler C?
• What if A was not defined?
• What if A worked differently — something like this one

There are so many ifs, it sounds like trouble, and that depending on one of the extensions yourself and taking the handler from there, is explicit and versioned and perhaps better?

[stevemk14ebr]

I actually don't care at all about extensions overriding each other. I only want the 'previous' handler before the user options.handlers override the registered (extensions + built-in) ones. For this both solutions I propose should work in all cases.

I.E 'if I were to override table via options.handlers, allow me to invoke the original logic that existed as if I did not provide a user options table handler'.

[wooorm]

I actually don't care at all about extensions overriding each other. I only want the 'previous' handler before the user options.handlers override the registered (extensions + built-in) ones. For this both solutions I propose should work in all cases.

That would fail in the case of my second and third examples though? Because either there would not be a table formatter, or a completely different table formatter was set as “previous”?
What is in the ...code modified tmp... in your original post? that might clarify to me what you’re doing.

---

From your previous comment:

My PR will make it so that all of the handlers registerd by extensions & default list are put on the key 'originalHandlers'. The user overrides are not put into this list due to this line:

I missed this, thanks for explaining

I’m not a fan of originalHandlers only working for user-provided options, extensions would expect them to work as well (but they don’t), I think that’s hard to explain/document/reason about.

And I think options itself should be treated the same as extensions — that options.handlers should be handled the same as extensions[x].handlers

[stevemk14ebr]

Here is what I actually do right now with the current live version, where none of the suggested features are available:

import { toMarkdown } from 'mdast-util-to-markdown';
import { toMarkdown as gfmTableToMarkdown } from 'mdast-util-gfm-table';
//...
handlers: {
                    table: (node, parent, context) => {
                        console.log(gfmTableToMarkdown);

                        // this original GFM handler does this
                        let tbl = gfmTableToMarkdown();
                        let txt = toMarkdown(node, { extensions: [tbl] });

                        // then we take the original and regex | in header to ||
                        txt = txt.replace(/\n(\|-*\|)(-*\|)*/, '');
                        let parts = txt.split('\n');
                        if (parts.length > 1) {
                            let header = parts[0].replaceAll("|", "||");
                            parts.shift();
                            return header + '\n' + parts.join('\n');
                        }
                        return txt;
                    },
}

As you can see I have to re-create the implementation of how the GFM plugin's toString works. I'd like instead to be able to write:

handlers: {
                    table: (node, parent, context) => {
                        // invoke original handler before user override
                        let txt = context.originalHandlers.table(node, parent, context);

                        // then we take the original and regex | in header to ||
                        txt = txt.replace(/\n(\|-*\|)(-*\|)*/, '');
                        let parts = txt.split('\n');
                        if (parts.length > 1) {
                            let header = parts[0].replaceAll("|", "||");
                            parts.shift();
                            return header + '\n' + parts.join('\n');
                        }
                        return txt;
                    },
}

You're correct this code would not work if the originalHandlers.table wasn't defined. However, my own pipeline knows that I use the GFM plugin so will exist. And this is all defined at the callsite so it's not hidden anywhere. This feature is up to the user to know in which context what type of handler is expected, any undefined keys or anything don't matter at all until a user tries to call them:

const jiraConverter = Remark().use([
            [RemarkGFM, { singleTilde: false, tableCellPadding: false }], RemarkBreaks, RemarkDirective,  [RemarkStringify, {
                handlers: {
                    table: (node, parent, context) => {
                              // ... use originalHandlers, can see whole pipeline right here during config

As an aside the GFM plugin would be awesome if it had a table header type so the user could customize the stringification of just the header (then I wouldn't have to do this at all!).

But I generally believe allowing the user to hook back into previous logic is important, this is how user GUIs commonly implement inheritance of events for example. Without a feature like this extendability of existing handlers is not really possible without re-creating the original implementation as my example above shows. Ex GUI event inheritance:

class Button {
public:      
        virtual handleClick() {
            std::cout << "Do normal button click handling" << std::endl;
        }
};

class MagicalCustomButton : public Button {
public:      
        virtual handleClick() {
            this.handleClick(); // call original handler
            std::cout << "I'm magic and do extra things after normal things" << std::endl;
        }
};

[stevemk14ebr]

I don't think it's reasonable to implement this feature to treat extensions the same as user options. To do this would require a stack/array that knows at which level is the current registered handlers. Each extension would have to then use the shared data context like this:
context.originalHandlers[context.level].table. This would require that the entire extension set is captured before a new one is rendered, then stored into this table forever. That's quite a bit of waste and complicates the data structures. It's also almost useless for extensions since they do not know what handlers exist (or their behavior/shape as you mention!) since they don't control the pipeline setup.

This isn't required if you treat just the user handlers as special, since there is only one level, the level just before when the user options.handlers are applied. And finally, only the user handlers know the full pipeline and the necessary context to use this feature.

[wooorm]

Thanks for your patience, I’ve been busy updating everything to ESM.

Your GUI events example, which you (and I) appreciate, is very close to what you in the next example explain as a wasteful, complex system. I agree that it is too complex, too. But it otherwise is a nice way of solving nesting. Events bubble up unless they’re handled.
Events work because it is unknown whether some other handler is attached or not.

In this case, you don’t want to bubble at all. You don‘t want remark-grid-tables to handle your table, because the code would crash as it can‘t make a Jira table out of those grid tables.
And you don‘t want an error if a user forgot to add mdast-util-gfm-table when generating Jira flavored markdown either.
I believe you instead (should) have a direct dependency on mdast-util-gfm-table’s handler.
And your example code can be nicely converted to:

import {gfmTableHandle} from 'mdast-util-gfm-table'

handlers: {
  table: (node, parent, context) => {
      let txt = gfmTableHandle(node, parent, context);
      // regexes
      return txt;
  },
}

But even better, you could publish this as mdast-util-jira-table, and a remark-jira plugin wrapper. Then anyone can use unified, remark, mdast, with your projects to generate Jira, without having to add remark-gfm.

I think an acceptable solution would be to start exposing handlers from packages, where possible?

[stevemk14ebr]

in the end that's what i did. I was hoping for a better interface though because I have to mimick how the plugin sets up its own handler and if that changes ill be chasing the interface. This might be the best way though, not sure I care much anymore since i've got it working ahah. Here is my total converter btw, actually ends up being pretty simple, except that i took a shortcut and used NodeToString which doesn't work in all cases properly with nested constructs (since custom jira stringifiers don't get called in NodeToString), but is good enough for me as most of my stuff is not nested:

const renderJira = (markdown) => {
        // convert all the markdown to jira

        const jirahandlers = {
            heading: (node, parent, context) => {
                let depth = node.depth;
                let value = NodeToString(node);
                return `h${depth}. ${value}`;
            },
            inlineCode: (node, parent, context) => {
                let value = NodeToString(node);
                return `{{${value}}}`;
            },
            code: (node, parent, context) => {
                let lang = node.lang;
                if (lang) {
                    return `{code:${lang}}\n${node.value}\n{code}`;
                }
                return `{code}\n${node.value}\n{code}`
            },
            horizontalRule: (node, parent, context) => {
                // always override with this, jira expects this
                return '----';
            },
            delete: (node, parent, context) => {
                let value = NodeToString(node);
                return `-${value}-`;
            },
            emphasis: (node, parent, context) => {
                let value = NodeToString(node);
                return `_${value}_`;
            },
            strong: (node, parent, context) => {
                let value = NodeToString(node);
                return `*${value}*`;
            },
            break: (node, parent, context) => {
                return "\n";
            },
            link: (node, parent, context) => {
                let url = node.url;
                node.url = "";
                let childContent = NodeToString(node);
                return `[${childContent}|${url}]`;
            }
        };

        const tableHandler = {
            table: (node, parent, context) => {
                // this original GFM handler does this
                let tbl = gfmTableToMarkdown();

                // table must use same handlers as rest of pipeline
                let txt = toMarkdown(node, {
                    extensions: [tbl], handlers: jirahandlers
                });

                // then we take the original and regex | in header to ||, as well as remove the | ---- | pattern below header
                txt = txt.replace(/\n\|\s+-*?\s+\|(\s+-*?\s+\|)*/, '');
                let parts = txt.split('\n');
                if (parts.length > 1) {
                    let header = parts[0].replaceAll("|", "||");
                    parts.shift();
                    return header + '\n' + parts.join('\n');
                }
                return txt;
            }
        };

        const jiraConverter = Remark().use([
            [RemarkParse, RemarkGFM, { singleTilde: false, tableCellPadding: false }], RemarkBreaks, RemarkDirective, [RemarkStringify, {
                handlers: { ...jirahandlers, ...tableHandler }
            }]
        ]);

[wooorm]

Seems the current state is okay-ish! Glad its working fine for now!