Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bjesuiter opened this issue Oct 4, 2021 · 9 comments
Closed

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

bjesuiter opened this issue Oct 4, 2021 · 9 comments
Labels
upstream Caused in upstream, not in this repo

Comments

@bjesuiter
Copy link

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
Copy link
Author

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

@yhatt
Copy link
Member

yhatt commented Oct 4, 2021

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

@yhatt yhatt transferred this issue from marp-team/marpit Oct 4, 2021
@yhatt
Copy link
Member

yhatt commented Oct 4, 2021

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

bjesuiter commented Oct 28, 2021

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
Copy link
Member

yhatt commented Oct 28, 2021

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 mathjax/MathJax-src#741.

@yhatt yhatt added the upstream Caused in upstream, not in this repo label Oct 28, 2021
@yhatt
Copy link
Member

yhatt commented May 11, 2022

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

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.

@yhatt yhatt closed this as completed May 11, 2022
@bjesuiter
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Caused in upstream, not in this repo
Projects
None yet
Development

No branches or pull requests

2 participants