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

gatsby-remark-images in MDX doesn't generate base64 placeholders #15058

Closed
robinmetral opened this issue Jun 23, 2019 · 30 comments · Fixed by #16062
Closed

gatsby-remark-images in MDX doesn't generate base64 placeholders #15058

robinmetral opened this issue Jun 23, 2019 · 30 comments · Fixed by #16062
Labels
type: bug An issue or pull request relating to a bug in Gatsby

Comments

@robinmetral
Copy link
Contributor

robinmetral commented Jun 23, 2019

Description

Using gatsby-remark-images in MDX (see this guide) doesn't generate a base64 placeholder.

➡️ demo repo ⬅️
➡️ live example ⬅️

Steps to reproduce

  1. create a new default gatsby site gatsby new gatsby-image-with-mdx
  2. cd gatsby-image-with-mdx
  3. install mdx and gatsby-remark-images yarn add gatsby-plugin-mdx @mdx-js/mdx @mdx-js/react gatsby-remark-images
  4. configure the plugin and filesystem source in gatsby-config.js:
    {
      resolve: `gatsby-plugin-mdx`,
      options: {
        gatsbyRemarkPlugins: [
          {
            resolve: `gatsby-remark-images`,
            options: {
              maxWidth: 600,
            },
          },
        ],
      },
    },
    // source posts 
    {
      resolve: `gatsby-source-filesystem`,
      options: {
        name: `posts`,
        path: `${__dirname}/src/posts`,
      },
    },
  1. create a post at /src/posts/testing-gatsby-image/index.mdx:
---
slug: testing-gatsby-image
---

# Testing gatsby-image in MDX

![An image](./image.png)
  1. Drop any image in the same directory (copy the astronaut for example) and rename it image.png
  2. Implement a simple gatsby-node.js:
const path = require("path")
exports.createPages = ({ graphql, actions }) => {
  const { createPage } = actions
  return new Promise((resolve, reject) => {
    resolve(
      graphql(
        `
          {
            allMdx {
              edges {
                node {
                  id
                  frontmatter {
                    slug
                  }
                }
              }
            }
          }
        `
      ).then(result => {
        // handle errors
        if (result.errors) {
          console.error(result.errors)
          reject(result.errors)
        }

        // create posts pages
        result.data.allMdx.edges.forEach(({ node }) => {
          createPage({
            // slug generated with createNodeField
            path: node.frontmatter.slug,
            // post template
            component: path.resolve(`./src/templates/post.js`),
            // pass id as context
            context: { id: node.id },
          })
        })
      })
    )
  })
}
  1. Add a template at /src/templates/post.js:
import React from "react"
import { graphql } from "gatsby"
import MDXRenderer from "gatsby-plugin-mdx/mdx-renderer"

const PostTemplate = ({ data }) => <MDXRenderer>{data.mdx.body}</MDXRenderer>

export const POST_QUERY = graphql`
  query POST_QUERY($id: String) {
    mdx(id: { eq: $id }) {
      body
    }
  }
`

export default PostTemplate
  1. run gatsby develop
  2. navigate to http://localhost:8000/testing-gatsby-image
  3. inspect the image

Expected result

The generated markup should include a base64 placeholder image for the "blur up" effect, like when using gatsby-remark-images with gatsby-transformer-remark.

Actual result

Generated markup:

<span class="gatsby-resp-image-wrapper" style="position: relative; display: block; margin-left: auto; margin-right: auto; max-width: 600px;">
  <span class="gatsby-resp-image-background-image" style="padding-bottom: 100%; position: relative; bottom: 0px; left: 0px; background-size: cover; display: block;"></span>
  <img class="gatsby-resp-image-image" style="width: 100%; height: 100%; margin: 0px; vertical-align: middle; position: absolute; top: 0px; left: 0px; box-shadow: white 0px 0px 0px 400px inset;" alt="An image" title="" src="/static/6d91c86c0fde632ba4cd01062fd9ccfa/34e8a/image.png" srcset="/static/6d91c86c0fde632ba4cd01062fd9ccfa/d4214/image.png 150w,/static/6d91c86c0fde632ba4cd01062fd9ccfa/135ae/image.png 300w,/static/6d91c86c0fde632ba4cd01062fd9ccfa/34e8a/image.png 600w,/static/6d91c86c0fde632ba4cd01062fd9ccfa/8ff1e/image.png 800w" sizes="(max-width: 600px) 100vw, 600px">
</span>

Environment

  System:
    OS: Linux 4.15 Ubuntu 18.04.2 LTS (Bionic Beaver)
    CPU: (4) x64 Intel(R) Core(TM) i5-4260U CPU @ 1.40GHz
    Shell: 4.4.19 - /bin/bash
  Binaries:
    Node: 10.9.0 - ~/.nvm/versions/node/v10.9.0/bin/node
    Yarn: 1.16.0 - /usr/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v10.9.0/bin/npm
  Languages:
    Python: 2.7.15+ - /usr/bin/python
  Browsers:
    Chrome: 67.0.3396.87
    Firefox: 67.0.2
  npmPackages:
    gatsby: ^2.10.0 => 2.10.0
    gatsby-image: ^2.2.1 => 2.2.1
    gatsby-plugin-manifest: ^2.2.0 => 2.2.0
    gatsby-plugin-mdx: ^1.0.5 => 1.0.5
    gatsby-plugin-offline: ^2.2.0 => 2.2.0
    gatsby-plugin-react-helmet: ^3.1.0 => 3.1.0
    gatsby-plugin-sharp: ^2.2.1 => 2.2.1
    gatsby-remark-images: ^3.1.0 => 3.1.0
    gatsby-source-filesystem: ^2.1.0 => 2.1.0
    gatsby-transformer-sharp: ^2.2.0 => 2.2.0
  npmGlobalPackages:
    gatsby-cli: 2.7.0
@vpicone
Copy link
Contributor

vpicone commented Jun 24, 2019

I believe gatsby-plugin-mdx is for gatsby v1. From the readme:

this plugin is only applied to gatsby v1. for v2 please use gatsby-mdx

For Gatsby v2, gatsby-mdx is the preferred plugin.

However, I'm currently experiencing this same issue with gatsby-mdx so I'm inclined to think this doesn't play a role in the issue.

@robinmetral
Copy link
Contributor Author

robinmetral commented Jun 24, 2019

@vpicone this was unclear to me as well, but with the plugin being integrated into Gatsby gatsby-mdx will be deprecated in favor of gatsby-plugin-mdx. See #15068 🙂

The upcoming merge of #15076 should make things clearer overall!

As you mentioned though, I don't think this is related to the package.

@ChristopherBiscardi
Copy link
Contributor

gatsby-plugin-mdx was the gatsby-1-compatible version and gatsby-mdx the 2.0 compatible version. Since I'm in the process of giving gatsby-mdx to the gatsby monorepo in #14657, we're also doing a rename with the gatsby-mdx 1.0 release. The future-facing package will be gatsby-plugin-mdx@1.0.

I'll make this clear when I write and release the official gatsby-mdx 1.0 blog post. Until then the gatsby-plugin-mdx package is basically "dark launched".

@robinmetral
Copy link
Contributor Author

Thanks for clarifying @ChristopherBiscardi, and thanks for your work on the amazing plugin! 🎉

...now about these missing base64 placeholders, any thoughts? 😛

@ChristopherBiscardi
Copy link
Contributor

@robinmetral I haven't had a chance to look into it yet since I just got back from vacation yesterday :) Thanks for this issue and the repro repo, that'll make it much easier to figure out what's going on.

@robinmetral
Copy link
Contributor Author

Thank you! Let me know if there's anything else I can do 🙂

@pedrolamas
Copy link
Contributor

Glad I found this already reported by @robinmetral as I was about to do it myself! 😛

@vpicone vpicone assigned vpicone and unassigned vpicone Jul 9, 2019
@vpicone vpicone added the type: bug An issue or pull request relating to a bug in Gatsby label Jul 15, 2019
@vpicone
Copy link
Contributor

vpicone commented Jul 15, 2019

This is likely related to #15486

The proposed solution there by @cwgw solves this problem.

The problem: gatsby-remark-x plugins that use the gatsby config files (like gatsby-remark-images) are not getting loaded properly by gatsby-plugin-mdx due to the gatsbyRemarkPlugins option not being processed correctly by gatsby.

workaround: supply the name of the plugin under a new plugins array in gatsby-plugin-mdx

{
  resolve: `gatsby-plugin-mdx`,
  options: {
    gatsbyRemarkPlugins: [
      {
        resolve: `gatsby-remark-images`,
        options: {
          maxWidth: 1152,
          linkImagesToOriginal: false,
          quality: 75,
          withWebp,
        },
      },
    ],
    plugins: ['gatsby-remark-images'],
  },
},

@cwgw
Copy link
Contributor

cwgw commented Jul 15, 2019

I'm not sure that this is related to #15486.

The problem there is that gatsby api files (like gatsby-browser.js and gatsby-ssr.js) within gatsby-remark-images don't seem to be loaded.

However, the base64 placeholder isn't generated in one of those gatsby api files. I think this issue may be something else.

@vpicone
Copy link
Contributor

vpicone commented Jul 16, 2019

@cwgw fair enough, adding it to the plugins array did bring back the blur up effect but could be unrelated cause

@codepunkt
Copy link

@ChristopherBiscardi I was just about to open an issue on this, but just found it has already been reported.
The branch remarkImages-md-vs-mdx on https://github.com/codepunkt/codepunkt.de also shows this problem with svg placeholders.

Steps to reproduce:

  1. check out the repo
  2. switch to remarkImages-md-vs-mdx branch
  3. yarn && gatsby build && gatsby serve
  4. click the articles link on the root page to get to /blog, compare the markdown and mdx posts

My debugging into gatsby-remark-images shows that the placeholders are created, the placeholder html with data-uri placeholders is set as node.value here:

https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-remark-images/src/index.js#L339

but i'm not sure i understand what's actually happening in that plugin and especially don't understand the interaction between markdownImageNodes and rawHtmlNodes. Maybe someone can walk us through this so we can get to the bottom of this together - or point us to documentation about what the plugin actually does 🙈

@cwgw
Copy link
Contributor

cwgw commented Jul 19, 2019

Hi @codepunkt.

Images in markdown can come in both markdown syntax and regular html. gatsby-remark-images has to distinguish between the two because they require different handling.

// markdown
![](./some-image.jpg)

// html
<img src="./some-image" />

The plugin traverses the tree looking for markdown image nodes and pushes each to the markdownImageNodes array, then does it again for all html (and jsx) nodes, adding them to rawHtmlNodes . It loops through both arrays, and replaces what it finds with the new markup it generates, ultimately returning an array that contains all of the nodes it modified.

HTML is complicated because images can be nested within other elements. So, the plugin uses cheerio to parse each html node and find and replace img elements.

@codepunkt
Copy link

codepunkt commented Jul 23, 2019

The markdownImageNode or rawHtmlNodes are found, but the generateImagesAndUpdateNode method bails out resolving to undefined because it can't find the appropriate imageNode in the files array.

Not sure where the getNodes method which is used to retrieve a list of all File nodes in get-source-plugins-as-remark-plugins.js (gatsby-plugin-mdx) is coming from and why it lists some files, but not others.

Will dig in deeper.

@codepunkt
Copy link

It seems the getNodes method is provided as part of the plugin API. There's two extension point used in my debugging runs, which are:

Not sure what i can do about that. Would be great if someone could chime in that knows how these things work.

@pedrolamas
Copy link
Contributor

The markdownImageNode or rawHtmlNodes are found, but the generateImagesAndUpdateNode method bails out resolving to undefined because it can't find the appropriate imageNode in the files array.

Just a thought, but can you ensure that all gatsby-source-filesystem plugin entries in the gatsby-config.js file appear before any other plugin? I had some issues where the files parameter would be missing files due to the order of the plugins.

@codepunkt
Copy link

All gatsby-source-filesystem plugin entries are before any other plugin. I have both a .mdx blog post and a .md blog post with the same markdown contents, each displaying an image placed relative to the md/mdx.

Plugin finds markdownImageNode for both, but bails out on mdx due to not having the relative image in the files array. Looks like i'm stuck here.

@pedrolamas
Copy link
Contributor

I've been debugging this via Visual Studio Code using my own site and I can see it does indeed retrieve the associated file, gets the base64 data and finally sets the AST node type to "html" and sets the value in it.

image

But somehow, seems that the background-image style gets lost and never gets to the final browser DOM...

@johno
Copy link
Contributor

johno commented Jul 23, 2019

Nice digging @codepunkt and @pedrolamas! My hunch is that the style attribute is being entirely dropped by React because it's formatted as a string rather than an object which is what JSX requires (which is what MDX is compiling to).

We have a babel plugin that handles some of coercion from HTML nodes to JSX, but it looks like this particular case is being missed.

@pedrolamas
Copy link
Contributor

I think we are getting somewhere now: that babel plugin uses the to-style to convert the string representation of the style attribute to a JSX object.

I need to run further tests on it, but this might be the culprit as I don't see where it handles the background-image value!

@pedrolamas
Copy link
Contributor

pedrolamas commented Jul 23, 2019

Confirmed: the issue is with the to-style package!

These are calls to that specific function, differing only that on the first you have "data:image" as part of the url, and the other "data_image" (so I removed the ":" from the url)

> toStyleObject("border-radius: 10px; background: red; background-image: url('data:image/base64,asdasdasdasdasdasdasdasdasdasdasdasaasdas')", {camelize: true})

{ borderRadius: '10px', background: 'red' }

> toStyleObject("border-radius: 10px; background: red; background-image: url('data_image/base64,asdasdasdasdasdasdasdasdasdasdasdasaasdas')", {camelize: true})

{ borderRadius: '10px',
  background: 'red',
  backgroundImage:
   'url(\'data_image/base64,asdasdasdasdasdasdasdasdasdasdasdasaasdas\')' }

As you can see, it successfully added the background-image on the second example, so the problem is here!

We fix the to-style package, we fix our problem!

@robinmetral
Copy link
Contributor Author

Amazing @pedrolamas, thanks for digging. Are you opening a PR?

@pedrolamas
Copy link
Contributor

@robinmetral I have a fix now, some more tests and will push a fix in a couple of hours!

@codepunkt
Copy link

codepunkt commented Jul 24, 2019

@pedrolamas @robinmetral @johno Seems like this is a problem that needs fixing. Great job!

However, the data-uri is never generated in my case with the plugin bailing out right before generating it because the file is not found in files array.

Here:
https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-remark-images/src/index.js#L127

A reproducible example is here:
https://github.com/codepunkt/codepunkt.de/tree/remarkImages-md-vs-mdx

@codepunkt
Copy link

codepunkt commented Jul 24, 2019

Cleaned my example branch up to be able to spot the problem easier.
Image file seems to exist in the files array (not sure why it didn't before), but the comparison fails because of windows drive letters.

The absolutePath properties of files in the files array look like this:

D:/web/codepunkt.de/src/pages/blog/mdx-test/bridge.jpg

The absolutePath and dir of a node retrieved by getNode look like this:

d:/web/codepunkt.de/src/pages/blog/mdx-test/index.mdx

The dir is used in line 114 to re-constuct the image path based on the relative image URL.

Because of this difference, the comparison between the actual file absolutePath and the re-constructed image-path in line 121 fails and no placeholder is generated.

Could this be a generic problem with windows paths/drive letters in Gatsby?

I could fix it in gatsby-remark-images directly, but the files array and the getNode method are provided by Gatsby plugin API.

@robinmetral
Copy link
Contributor Author

@codepunkt I do know that Gatsby on Windows is not as seamless as on Unix. If you think this might be a broader problem, it might deserve its own issue!

@codepunkt
Copy link

codepunkt commented Jul 24, 2019

I don't even know where to begin. Whenever windows drive letters are in paths, Gatsby has to either lowercase them or uppercase them, but be consistent about it. I know it has also been a problem in node and has occured in webpack, yarn and lots of other tools.

As of now, i'd just like placeholder images 😭

@pedrolamas
Copy link
Contributor

pedrolamas commented Jul 24, 2019

So here's the situation: the dependency on to-style package is causing this problem! It basically doesn't know how to handle this type of information.

There's an open issue on this matter, but seems to me that this codebase is abandoned (there are open PR's from 2017!)

I'd recommend in using postcss to properly parse the css string (I've checked and it parses the base64 information without no issues)

I've found 2 places where this package is in use:

While we can use postcss on the first one (that's inside Gatsby repo), but it's the second one entry that we need to change to fix this issue!

Ideally, this should be fixed on the to-style repo but I'm not sure that the repo owner is still around...

@pedrolamas
Copy link
Contributor

I've made a PR that replaces to-style with style-to-object. From my tests, this fixes the main issue which was the base64 data not being output.

Smooth transitions might still be failing but I was going to leave that for when this gets approved/merged in the master branch.

@robinmetral
Copy link
Contributor Author

Looking great @pedrolamas, looking forward to seeing this merged and fixed! Thank you 🚀

@pedrolamas
Copy link
Contributor

My PR has shipped in the latest Gatsby and MDX dependencies, and from my tests, seems this issue is no more! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants