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

Support prepending of base url in markdown image paths #2724

Closed
aeneasr opened this issue May 7, 2020 · 24 comments · Fixed by #3313
Closed

Support prepending of base url in markdown image paths #2724

aeneasr opened this issue May 7, 2020 · 24 comments · Fixed by #3313
Labels
feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future.

Comments

@aeneasr
Copy link
Contributor

aeneasr commented May 7, 2020

🚀 Feature

I want to display images without using JSX in my Markdown code

![my image](/static/img/...)

and have Docusaurus modify the image src using the project's baseUrl.

Have you read the [Contributing Guidelines on issues]

Yes

Motivation & Pitch

Currently, it's recommended to link to an image using:

import useBaseUrl from '@docusaurus/useBaseUrl'

<img alt="ORY Oathkeeper with Prometheus and Grafana" src={useBaseUrl('img/docs/grafana.png')} />

This has several disadvantages. First it doesn't work with native markdown and thus looks confusing when browsing the docs on e.g. GitHub (both in preview and raw view!):

Bildschirmfoto 2020-05-07 um 11 12 49

Bildschirmfoto 2020-05-07 um 11 12 53

Second it's hard to understand for people who don't know Docusaurus. None of our docs contributors has figured out how to use this correctly. This means that the time to merge for docs changes goes up and frustrates both maintainers as well as contributors.

Third it breaks previews. Assuming that - as it's documented - the import is always the first element, we get previews in Slack

Bildschirmfoto 2020-05-07 um 11 15 52

and even the HTML meta tags which is a SEO-killer:

Bildschirmfoto 2020-05-07 um 11 16 27

Please consider adding this functionality to Docusaurus. I'd be happy to help!

@aeneasr aeneasr added feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. status: needs triage This issue has not been triaged by maintainers labels May 7, 2020
@yangshun
Copy link
Contributor

yangshun commented Jun 5, 2020

To get around this, you can override the description field in the front matter of your markdown docs. @lex111 has also worked on stripping import statements from the docs so this problem might not exist now. But adding support for prepending markdown paths with the baseUrl would be useful, possibly via a new syntax.

@yangshun yangshun removed the status: needs triage This issue has not been triaged by maintainers label Jun 5, 2020
@yangshun yangshun changed the title Support base url in markdown image paths Support prepending of base url in markdown image paths Jun 5, 2020
@jknoxville
Copy link
Contributor

Hey @yangshun I'd be happy to work on this, but since the syntax of markdown is already pretty well defined, it would be a shame to have to use a different one.

Would it be out of the question to change the default to always prepend the baseUrl? I imagine it's much more common than wanting to link to "outside" of the docusaurus site. You could still do that by using tags like the example above if you want to.

Or if not that, maybe an option you can set (possibly in docusaurus.config.js) to toggle this behaviour?

@aeneasr
Copy link
Contributor Author

aeneasr commented Jul 31, 2020

Linking to external images would work regardless of the automatic pre-use of useBaseUrl: https://my-domain/my-image.png. I don't think there's actually a use case for not base-url'ing the images given of how static files are loaded. I haven't used a single image reference without this hack (it is a hack because it's not native markdown) yet, and that includes hundred pages of docs :)

@slorber
Copy link
Collaborator

slorber commented Jul 31, 2020

Hey, this issue is already solved imho, you can use relative image paths (and colocate the image to its doc too, if needed):
https://v2.docusaurus.io/docs/markdown-features/#image-assets

It uses webpack file-loader under the hook, which also provides nice benefits: we can now detect bad image paths at compile time, + add a hash to the image URL so that it can be cashed more aggressively.

I don't like useBaseUrl either, and would like to deprecate it.

Can we close this PR?

@jknoxville
Copy link
Contributor

Ah ok, thanks, I'll give that a go :)

I was trying to use relative images today but didn't realise you had to import them, so the build would fail unless I prepended them with filepath://.
It's not directly related but that error message could use some improvement. It just said something like "can't resolve module url-loader". I can imagine a lot of people hitting that.

@slorber
Copy link
Collaborator

slorber commented Jul 31, 2020

hmmm, this is surprising because url loader is a core dependency, and is indeed a required dep for this feature to work.

Are you using something like yarn 2, pnp or something else? Did do yarn install? Can you run yarn why url-loader?

It may be because one of our package does not expose directly it as a direct dependency. Was precisely working on a bugfix of this part, we'll see in next release if it works better for you.

@aeneasr
Copy link
Contributor Author

aeneasr commented Aug 3, 2020

Yup, this is apparently solved indeed. We can close it IMO :)

@slorber
Copy link
Collaborator

slorber commented Aug 3, 2020

@jknoxville is it working for you? How did you find about filepath:// by reading the source code?

@jknoxville
Copy link
Contributor

@slorber sorry for the delay getting back to you. This is the build error I get if I use a markdown image without importing that particular image with both the following syntax:

![](./poisson.png)
![](poisson.png)

(where poisson.png is a file in the same dir as the markdown file referring to it)

../docs/overview/quick_start/quick_start.md
Module not found: Can't resolve 'url-loader' in '/data/sandcastle/boxes/fbsource/fbcode/beanmachine/docs/overview/quick_start'
Client bundle compiled with errors therefore further build is impossible.
../docs/overview/quick_start/quick_start.md
Module not found: Error: Can't resolve 'url-loader' in '/data/sandcastle/boxes/fbsource/fbcode/beanmachine/docs/overview/quick_start'
 @ ../docs/overview/quick_start/quick_start.md 1:5502-5610
 @ ./.docusaurus/registry.js
 @ ./node_modules/@docusaurus/core/lib/client/exports/ComponentCreator.js
 @ ./.docusaurus/routes.js
 @ ./node_modules/@docusaurus/core/lib/client/clientEntry.js
 @ multi ./node_modules/@docusaurus/core/lib/client/clientEntry.js
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

yarn why url-loader

Using Yarn from /data/sandcastle/boxes/fbsource/xplat/third-party/yarn/yarn
yarn why v1.17.0-20190429.1820
[1/4] Why do we have the module "url-loader"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "@docusaurus#core" depends on it
   - Hoisted from "@docusaurus#core#url-loader"
   - Hoisted from "@docusaurus#preset-classic#@docusaurus#plugin-content-blog#@docusaurus#mdx-loader#url-loader"
info Disk size without dependencies: "492KB"
info Disk size with unique dependencies: "596KB"
info Disk size with transitive dependencies: "3.02MB"
info Number of shared dependencies: 11
Done in 1.02s.

I'm using yarn 1.17.0, the one that runs as standard at facebook.

I think the filepath:/// thing was a red herring, I found it in an issue by googling the error message, and thought it might be the new required syntax so gave it a try.


So speaking for myself, I still can't get any images to work using the markdown syntax.
Maybe I'm doing something wrong, but as far as I can tell I'm following the docs.

Tried on versions 58 and 61.

To clarify, I'd be happy with the restriction to only use absolute paths instead, but they don't currently get the baseUrl prepended.

@slorber
Copy link
Collaborator

slorber commented Aug 5, 2020

This is weird
I have no idea what's going on but it's supposed to work.

These features are new so don't stay on alpha 58.
Are you sure you have all your deps on Docusaurus alpha 61?
May be worth to inspect your lockfile, or try regenerating it.

What gives yarn why @docusaurus/core ?
What gives yarn why @docusaurus/mdx-loader ?

Can you create a small repro?

Maybe try to remove the .docusaurus folder?
This one too website/node_modules/.cache/cache-loader

@jknoxville
Copy link
Contributor

Thanks for the help @slorber

I regenerated the yarn.lock, and verified that it only contains v61 of any docusaurus packages, still the same thing.

However, I tried out using relative markdown images on the docusaurus website itself, and it works, so I can agree with you that relative images do in fact work and this issue can be closed :) there's just clearly something wrong with this other docusaurus installation, but as far as I know thats unrelated to this issue.

@slorber
Copy link
Collaborator

slorber commented Aug 6, 2020

Great to know :)

This is weird that it doesn't work on your website. Unfortunately without a repro it will be hard to troubleshot.

@slorber slorber closed this as completed Aug 6, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

@jknoxville I think @anshulrgoyal found out why the url loader does not work in your case, because the docs are outside of the website, and it does not resolve against parent/website/node_modules but against parent/node_modules.

Adding URL loader in the parent is likely to solve this issue, but we'll also add resolution to website/node_modules, as it's likely to be useful for those migrating from docusaurus v1 and keeping docs in place.

I've seen this on Flipper: https://github.com/facebook/flipper/blob/master/website/docusaurus.config.js#L118

Did you migrate from v1 and decided to keep the docs in place instead of moving them?

@jknoxville
Copy link
Contributor

Did you migrate from v1 and decided to keep the docs in place instead of moving them?

Yes exactly, I didn't see a reason to move them, but can do if it's necessary. I'd prefer that than having to turn the parent into a JS package.

@slorber
Copy link
Collaborator

slorber commented Aug 19, 2020

No it should be supported, we don't want to force users to move folders as it seems it can messup a bit the git history 🤪 we are currently experiencing that in RN website migration

@dulinriley
Copy link

dulinriley commented Feb 3, 2021

@slorber I'm running into this issue as well for the Hermes website, using "@docusaurus/[email protected]". Tried upgrading to alpha.70 as well and it didn't fix the issue.

Hermes's docusaurus directory is here: https://github.com/facebook/hermes/tree/master/website
It's docusaurus.config.js has a docs section that points to ../doc, where we store our Markdown files.
I tried to add an image and use a Markdown relative import:

![My image](./foo.png)

And it had the same 'url-loader' module error mentioned above.

Has this "not subdirectory of website" issue been fixed yet? Has an updated docusaurus been pushed?

@slorber
Copy link
Collaborator

slorber commented Feb 4, 2021

Hi @dulinriley

I don't see the /doc folder in that repo 😅

If the image is in /static, my reco would to to avoid relative paths. These paths are useful when you want to colocate assets close to the doc (possibly translate/vesion them alongside the markdown files).

If a file is in /static/img/foo.png, you should just use /img/foo.png and it should work in all cases.

I can give more advice if you give me more concrete file URLs to look at, or create a repro using http://new.docusaurus.io/

@dulinriley
Copy link

I don't see the /doc folder in that repo

The link is to a subdirectory of the Hermes repo. If you go one directory up from website you'll see doc as one of the subdirectories.

I can try putting them in the static directory instead and see if that fixes my problem

@dulinriley
Copy link

I moved the images to the /static directory but that didn't solve the issue, I still get:

./docs/IR.md
Module not found: Can't resolve 'url-loader' in '/Users/dulinr/fbsource/xplat/hermes/website/docs'

That's adding the following to the docs/IR.md file:

![Test](/img/favicon.ico)

(and after moving the directory doc to website/docs)

@slorber
Copy link
Collaborator

slorber commented Feb 5, 2021

@dulinriley can you show me a failing PR/branch so that I see what you are attempting to do exactly?
Give me a link to the exact image + the doc of that PR, and I'll tell you what to do
It's hard to debug something remotely that I can't inspect 😅

@dulinriley
Copy link

dulinriley commented Feb 9, 2021

@slorber I made a PR for this: https://github.com/dulinriley/hermes/pull/1

That PR contains the error message. For that PR I also tried moving the doc directory underneath the website directory and renaming to "docs" but that didn't fix it.

@slorber
Copy link
Collaborator

slorber commented Feb 10, 2021

Hi @dulinriley

You need to run yarn add url-loader -D to the website folder and it works.

I think it fails because the md image paths are converted require() calls, needing node_modules/url-loader to exist, but it does not exist because it's just a transitive dependency of Docusaurus. Will try to see how to fix that.

@dulinriley
Copy link

Is it just a dev dependency, or is it needed for the production website build?

@slorber
Copy link
Collaborator

slorber commented Feb 10, 2021

yes sorry it is useful to run yarn build if you build on the CI (so remove -D)
(btw you have file-loader as dev dependency, the same apply)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants