marp-team / marp-core

The core of Marp converter
MIT License
766 stars 129 forks source link

Type Error after trying to bundle marp-core with rolllup #259

Closed bjesuiter closed 2 years ago

bjesuiter commented 2 years ago

Hi Marp Team,

I currently try to implement your marp-core as a plugin for Obsidian. My Repo is here: https://github.com/bjesuiter/obsidian-marp-presentations (Only for reference)

The problem: When my Plugin file loads, it throws this error

TypeError: Cannot read property 'MathtoolsMethods' of undefined

Here is a screenshot of the error

Bildschirmfoto 2021-10-04 um 18 19 07

My Assumption: The error comes from something missing in my rollup pipeline, so that bundling breaks. Reason: it works before the latest commit: My Commits My Latest Commit

Since I don't use my own markdown currently, it can't be an error with that.

Thank you for reading my problem and hopefully you can help me solve it, since I'm relatively lost here :/

bjesuiter commented 2 years ago

Sorry, I realized that I created this issue in the wrong repo. Should I move it?

yhatt commented 2 years ago

No worry; I'll move this to Marp Core 🚚

yhatt commented 2 years ago

MathtoolsMethods seems coming from MathJax. Did not rollup emit circular dependency warnings when building? rollup may not be able to resolve CommonJS export correctly if there is circular dependency in a module. As the result, sometimes it might throw undefined property error just like that.

Marp team is bundling Marp Core through webpack in sub-projects like marp-vscode, and it works well. may have to look into bundling issue on rollup.

bjesuiter commented 2 years ago

Ahh, interesting! When I build in verbose mode, I see a circular dependency warning, as you expected!

(!) Circular dependenciestiming npm:load Completed in 31ms
node_modules/mathjax-full/js/input/tex/TexParser.js -> node_modules/mathjax-full/js/input/tex/ParseUtil.js -> node_modules/mathjax-full/js/input/tex/TexParser.js
node_modules/mathjax-full/js/input/tex/TexParser.js -> node_modules/mathjax-full/js/input/tex/ParseUtil.js -> /Users/bjesuiter/Develop/obsidian/obsidian-marp-presentations/node_modules/mathjax-full/js/input/tex/TexParser.js?commonjs-proxy -> node_modules/mathjax-full/js/input/tex/TexParser.js
node_modules/mathjax-full/js/input/tex/mathtools/MathtoolsMethods.js -> node_modules/mathjax-full/js/input/tex/mathtools/MathtoolsUtil.js -> node_modules/mathjax-full/js/input/tex/mathtools/MathtoolsMethods.js
...and 5 more
bjesuiter commented 2 years ago

Another Update:

It seems that I can add the mathjax module as dynamicRequireTarget, which fixes the circular dependency problem (e.g. I can compile without a warning):

dynamicRequireTargets: [
                // Inside mathjax is the following circular dependency:
                // node_modules/mathjax-full/js/input/tex/TexParser.js
                // -> node_modules/mathjax-full/js/input/tex/ParseUtil.js
                // -> node_modules/mathjax-full/js/input/tex/TexParser.js
                // This simulates a dynamic 'require' environment for the whole mathjax library to resolve the problem
                `node_modules/mathjax-full/**/*.js`,
            ],

The problem is now, that the imports do not seem to be working.

Bildschirmfoto 2021-10-04 um 22 25 41

I will play around with these values a bit, maybe I can make it work.

BTW: Thank you so much for looking into it that fast! <3

bjesuiter commented 2 years ago

Soo, i've toyed with this a lot, but unfortunately, I couldn't get it to work :/

The problem really seems to be that marp/mathJax contains these circular imports. I can't avoid it with rollup, since the bundle is broken afterwards. I can't switch to WebPack easily, since rewriting the whole rollup config for Obsidian Plugins to webpack is not feasible.

My solution would be to use the CLI Version of MARP, but this might be slower and produces a lot of coding overhead, since I have to generate a real html file from the absolute path of the input markdown and load it back into obsidian via file handling.

And in theory I would have to regenerate it on each change in the source markdown, which would potentially slow down anything.

Do you have any idea, how we could solve this? My Idea: @yhatt Could you, for example, compile marp-core to a single-file ESM Bundle with your webpack config, so that these cyclic dependency problems get resolved on your pipeline and I can import it simply as bundle?

yhatt commented 2 years ago

Having circular dependencies is potentially a bug of MathJax so the best for everyone may become the thing that MathJax was patched to fix that. And fortunately, it seems to be in progress at https://github.com/mathjax/MathJax-src/pull/741.

yhatt commented 2 years ago

Now obsidian-sample-plugin is using esbuild instead of rollup, and it can bundle Marp Core straightly. Marp is available in Obsidian :point_down:

It is good for developers to know the thing that rollup cannot bundle Marp Core due to MathJax's circular dependency.

We close this because should keep cleaning up the issue tracker to focus Marp improvements.

bjesuiter commented 2 years ago

Thank you very much for looking into it, i unfortunately haven't came around to implement the new things yet. But it's nice to now that they're available! I think, my Issue will be resolved with this now. Thanks again!