-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
ESM Support - Challenge #5758
Comments
The main issue that is preventing the latest versions of Mermaind (that uses ESM modules) is that MkDocs doesn't support them. Once MkDocs supports them, themes like Material can then add support. See relevant discussion: #5708. |
correct me if I am wrong but wasn't support added in 1.5.0? |
Yes, but there are no releases yet for 1.5.0. And once there is, there will need to be work here to add support. I'm also not sure of the timeline for getting support into Material either, only that I believe there are plans to get it in sometime after 1.5.0. |
Thanks for reporting. We understand the issue and acknowledge the issue. We're not really blocked by MkDocs 1.5.0, because Material for MkDocs auto-loads Mermaid.js once it is needed. This is done here. More specifically, here: mkdocs-material/src/assets/javascripts/components/content/code/mermaid/index.ts Lines 70 to 74 in 3e8faa0
... and here: mkdocs-material/src/assets/javascripts/components/content/code/mermaid/index.ts Lines 87 to 133 in 3e8faa0
We're currently very busy with our current docs restructuring, as well as finishing the release of 9.2.0 and the new search integration. We'd be very happy for help here and we'd love to accept a PR to help upgrade our Mermaid.js integration to ES modules with a fallback for browsers that don't support it. We'll also be happy to give assistance in a PR, if it's necessary. Note that ESM integration seems to be rather different in implementation. We're also okay with dropping some of the version range of the browsers we currently support if it's tiny enough, but this needs more data and further re-evaluation. Please note that some users are stuck on older systems and if we can offer a "low-cost" fallback, it's definitely the way we should go. This is at least what Material for MkDocs has done in the last years. So, @elliot-huffman, are you up to it? 😊 |
@squidfunk, I can try as I am a typescript guy, it won't be fast since my day job is a different TypeScript project. There is a polyfill available in this module but that would require an additional package for this project to build successfully. Marketshare Reference: You target 93% of the MarketShare currently according to your browser support docs page, so this should be compatible with your current target without needing a polyfill to add support for older browsers from my point of view. |
Your help is very much appreciated!
Yes, I agree, no polyfill if possible, as we want to keep the JS we ship as small as possible, only loading what's necessary. But we could keep the old way if it turns out to be necessary if we'd be losing significant support range. To assess that, we'd need to update the numbers to find out whether we can drop Chrome 49-60 for instance.
Agree. Note that the 92% in our browser support matrix should be updated, as those numbers were last checked in January 2022, so we're likely past the 95% now. In any case, we need to update the numbers. |
Any progress on this? Would love to release it as part of 9.2.0 in the coming weeks. |
I will work on it soon if you are ok with the ESM model. |
Yes, but with a commonjs fallback for Mermaid.js. |
Are there any updates on this, @elliot-huffman ? Curious as we're having some other issues that are depending on a PR on this issue :) (see #5859) |
Sorry about not catching this, I will get to work on it as soon as my current company product feature is completed. That should be within the next 2 weeks. |
I've successfully utilized the mkdocs-mermaid2-plugin to resolve the versioning issue with Mermaid 10+. I hope this information can be of help to you all. However, it's crucial to note that when configuring the superfence, one needs to modify the function used for the
|
I'd like to take this and migrate the build system to parcel v2, since it looks promising for the features we use: We could also take this chance to minimize external deps, remove shimmed polyfills and remove |
Thanks for suggesting. However, you're proposing very drastic changes. We're very happy with our current homegrown build system that is based on RxJS. It allows us a custom workflow without fighting with APIs from other tools. Previously we had Webpack, and we removed it for a reason. That being said, the |
That is precisely the problem I'm facing. The current build system is hard to customize / plug external steps, and the integrations are not very good.
I think a build step is necessary, as that's the whole point if you are installing from the source. You should use a release if you'd like to use prebuilts. If you don't want a system-wide build time requirement on Node.js, consider https://github.com/ekalinin/nodeenv. |
holey crap, my time has just slipped away, I am sorry I have not been able to get to this sooner. I am still not quite available to work on this feature. As soon as I get some spare time I will get this implemented. I have not forgotten about this! |
This should be on its own issue. This is off topic. |
Yes, I agree, it's definitely off-topic. Please don't discuss the build system here. It has absolutely nothing to do with adding support for ES modules. You can open a discussion, if you wish to discuss. |
My opinion is that differential bundling is vital for supporting native ES modules without sacrificing legacy browser (e.g. Chrome 49-60, Firefox 53-59, Opera 36-47) support. |
I follow because I need new mermaid version 10 for fixing some issue in my workflow diagram. |
There's currently an open PR at #6265. I just need some time to review it. If you can't wait, you can use the changes introduced in this PR right now and give feedback here whether it solves your problems |
Fixed in #6265. |
Released as part of 9.4.13. |
Context
The statement in the old issue is that it will hurt browser support. I would like to challenge this.
Description
According to can I use (the same database you use, a great one)
https://caniuse.com/es6-module, if you enable support for this, you are still targeting 95%+ of all users, up by 3% from your target https://squidfunk.github.io/mkdocs-material/browser-support/ of 92%.
You are also putting your project at risk because security issues that have been identified and fixed on Mermaid.JS are not being implemented on your project:
e.g. mermaid-js/mermaid#847 (this example is where more bypasses were found for the old issue and were patched again to prevent more bypasses.)
Related links
#5193
Use Cases
Keeping Deps Up to date and reduce project complexity by allowing to update more deps to the latest version and use newer tech.
Visuals
No response
Before submitting
The text was updated successfully, but these errors were encountered: