Skip to content
New issue

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

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

Already on GitHub? # to your account

Open Graph images can be inserted as base64, preventing social previews #9345

Closed
7 tasks done
Zwyx opened this issue Sep 26, 2023 · 7 comments · Fixed by #9369
Closed
7 tasks done

Open Graph images can be inserted as base64, preventing social previews #9345

Zwyx opened this issue Sep 26, 2023 · 7 comments · Fixed by #9369
Labels
bug An error in the Docusaurus core causing instability or issues with its execution domain: markdown Related to Markdown parsing or syntax

Comments

@Zwyx
Copy link
Contributor

Zwyx commented Sep 26, 2023

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

When building the site, some og:image are inserted as base64 data, instead of as a URL.

This prevents the image from being visible when sharing the blog post on Twitter, Slack, Signal, etc.

Example:

<meta data-rh="true" property="og:image" content="data:image/jpeg;base64,...">

which makes the image not visible in social previews (testable with opengraph.xyz)

<meta data-rh="true" property="og:image" content="https://zwyx.dev/assets/images/...">

which makes the image showing up correctly (opengraph.xyz).

Code available at github.com/zwyx/zwyx.dev.

Reproducible demo

https://github.com/Zwyx/zwyx.dev

Steps to reproduce

Please see description.

Expected behavior

Please see description.

Actual behavior

Please see description.

Your environment

  • Public source code: https://github.com/Zwyx/zwyx.dev
  • Public site URL: https://zwyx.dev
  • Docusaurus version used: 2.4.1
  • Environment name and version (e.g. Chrome 89, Node.js 16.4): Chrome 117, Node 18
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): Ubuntu 22.04 LTS + GitHub Actions

Self-service

  • I'd be willing to fix this bug myself.
@Zwyx Zwyx added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Sep 26, 2023
@thats-sarthak
Copy link

Can you please explain a little bit more about this issue

@Josh-Cena
Copy link
Collaborator

@thats-sarthak, the issue is that the image front matter key is bundled, seen here:

/**
* Converts assets an object with Webpack require calls code.
* This is useful for mdx files to reference co-located assets using relative
* paths. Those assets should enter the Webpack assets pipeline and be hashed.
* For now, we only handle that for images and paths starting with `./`:
*
* `{image: "./myImage.png"}` => `{image: require("./myImage.png")}`
*/
function createAssetsExportCode(assets: unknown) {

To fix this, we would probably need to use file-loader instead of url-loader, because the latter "smartly" inlines when the size is lower than a threshold. @slorber, do you have better ideas on how to handle this, since MDX loader should probably not be aware of which loader to use?

@Josh-Cena Josh-Cena added domain: markdown Related to Markdown parsing or syntax and removed status: needs triage This issue has not been triaged by maintainers labels Oct 1, 2023
@slorber
Copy link
Collaborator

slorber commented Oct 2, 2023

I didn't try but we already have something like that while processing Markdown images so that they don't get unexpectedly converted to ideal images.

I guess we could reuse the same logic:

const {
  loaders: { inlineMarkdownImageFileLoader }
} = getFileLoaderUtils();

const requireString = `${inlineMarkdownImageFileLoader}${escapePath(
  relativeImagePath
) + search}`;

attributes.push({
  type: "mdxJsxAttribute",
  name: "src",
  value: assetRequireAttributeValue(requireString, hash)
});

Let me know if you want to send a PR

@Zwyx
Copy link
Contributor Author

Zwyx commented Oct 3, 2023

Hey Seb! Thanks for the info, I'm keen to send a PR, give me a couple days ;-)

@Zwyx
Copy link
Contributor Author

Zwyx commented Oct 3, 2023

I've tried two things:

  • Use assetRequireAttributeValue as you suggested @slorber (if I understood correctly), however it doesn't change anything, the returned string is exactly the same:
require("!/xxx/docusaurus/node_modules/url-loader/dist/cjs.js?limit=10000&name=assets/images/[name]-[contenthash].[ext]&fallback=/xxx/docusaurus/node_modules/file-loader/dist/cjs.js!./typesafe-translations.jpg").default

This is done in this commit.

  • Completely remove url-loader from inlineMarkdownImageFileLoader. That fixes the issue, however it creates a lot of change: many images that were in build/assets/images are now in directly in build. Is that ok?

This is done in this commit.

@slorber
Copy link
Collaborator

slorber commented Oct 3, 2023

Yes indeed it doesn't work you'd need to create a new inline loader string that only uses file-loader and not url-loader. You can name it inlineMarkdownAssetFileLoader and use it inside loader.ts

Please submit a draft pr, it's easier to review than individual commits

@Zwyx
Copy link
Contributor Author

Zwyx commented Oct 4, 2023

Thanks @slorber, draft PR created here 👍

All works well. Only thing I'm thinking is that we might want to give another name to the new folder, because assets leads to a repetition: build/assets/assets. Let me know what you think.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution domain: markdown Related to Markdown parsing or syntax
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants