-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
onBrokenMarkdownImagePaths to catch bad paths without failing build #3992
Comments
I think this is remark or something like it. Either way, the setting is supposed to check for broken links, not no links at all. Not sure what should be done here. |
Hi, You upgrade from alpha 58 to 70, there were quite a few changes to make Docusaurus more "fail fast" recently, preventing you from deploying broken links, image paths, and using bad config. It is better to discuss the issues you have one by one.
Regarding empty markdown links and image URLs error, the behavior is the expected one to me. The "URL is mandatory" error is not related to the Does it make sense? I'm not against allowing empty URLs again if you present me a decent use-case for allowing empty urls |
Hi @slorber, I think the root of the issue comes down to how "broken" markdown links and image URLs/paths are defined and eventually handled by D2. Given the following examples:
and
Technically, both are broken markdown links but one of them (the empty URL/path) is a more egregious misuse/abuse of the markdown syntax. I can understand the reason for checking if the URL/path is present before attempting to resolve it, but I'm not 100% sure empty URLs/paths need to be handled in a completely separate way. When I first saw the error, I immediately set I guess what I'm proposing is that empty URLs/paths be handled within the same scope as the I agree with you in that there isn't a legitimate use case for allowing empty URLs/paths but I could make the same argument for having broken markdown links in general. I think the more important thing to accept is that broken markdown links happen, and it's typically not intentional. For example (In the case of our site), the content itself is being pulled in from an external repo that accepts open source contributions. Basically, some CI/CD automation pulls and generates docs (from this external repo) as part of the D2 build. I believe that these contributors might be following a predefined template that ends with asking for an image, which, in some cases, could be intentionally left blank as a "placeholder" of sorts. I'm not saying this is a legitimate use case - just highlighting how mistakes can and will happen. |
A missing image path/url does not really look related to a markdown link to me. Is it really a good idea to try to use this option for any markdown-related issue we have? Not sure. Do we also want another option just for missing image URLs? Not sure either. What I know is that Docusaurus should nudge users to do the right thing, and have non-empty markdown link/image urls/paths seems appropriate. If this is really important for you to support empty paths, why not building a remark plugin to support your specific usecase? You'd be able to tell the plugin to replace all empty tags by something else that we would allow, just as This should not be too complicated to write such logic, and we could even document this better on Docusaurus site? Here's a pseudo implementation, didn't test but it shouldn't be much more complicated than that remark plugin: const visit = require('unist-util-visit');
const path = require('path');
const plugin = () => {
const transformer = (root) => {
visit(root, 'link', (node) => {
if ( !node.url ) {
node.url = "#";
}
});
visit(root, 'image', (node) => {
if ( !node.url ) {
node.url = "/img/placeholder.png";
}
});
};
return transformer;
};
module.exports = plugin; |
Hi @slorber, thanks so much for taking the time to respond and for sharing the plugin sample code - I will definitely give it a try. Thanks! |
Please let me know how it works for you. Many users probably assume it's too complex to build such a remark plugin, and we should definitively highlight it's not complicated to build custom markdown syntax and document this better ;) |
Hi @slorber, I tested the remark plugin you suggested and I'm stilling seeing the same |
@sserrata if you use the Having a repro would be easier for me to be able to help you. I think I got it working in codesandbox: https://codesandbox.io/s/trusting-firefly-ku6ym?file=/docusaurus.config.js |
Hi @slorber, thanks for that tip. The build seems to be getting further along with
I currently have |
@sserrata as I said We could as well add a I'm ok to add more setting to customize these error cases and make them fail-safe but we need some additional docusaurus infra to apply these settings more reliably to all plugins (not just docs) without duplicating code. Note you can still hardcode a special case in your remark plugin to handle this specific broken image:
It won't prevent future commits to break the site but at least you have a way to upgrade without changing your docs. |
Hi @slorber , our team would certainly be interested in seeing support for |
🐛 Bug Report
The latest version of D2 fails to build if markdown links or images contain an empty URL value. A build may also fail if a relative URL value path cannot be resolved.
Have you read the Contributing Guidelines on issues?
Yes.
To Reproduce
(Write your steps here:)
Expected behavior
Inherit the
onBrokenMarkdownLinks
setting. For example, if set towarn
then error should be displayed as a warning and dev server or build should continue.(Write what you thought would happen.)
Actual Behavior
Seeing the following errors:
Error: Markdown link url is mandatory.
Error: Markdown Image url is mandatory.
(Write what happened. Add screenshots, if applicable.)
Your Environment
Reproducible Demo
demisto/content-docs#429
The text was updated successfully, but these errors were encountered: