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-plugin-mdx]: Double images / blurry / wrong style #15486

Closed
rwieruch opened this issue Jul 7, 2019 · 33 comments
Closed

[gatsby-plugin-mdx]: Double images / blurry / wrong style #15486

rwieruch opened this issue Jul 7, 2019 · 33 comments
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@rwieruch
Copy link
Contributor

rwieruch commented Jul 7, 2019

I am using gatsby-remark-images in my Gatsby MDX powered website. Not sure whether this was introduced during the last months, because it worked before, but after upgrading everything I have to set the following style manually to override the responsive Gatsby style to make it work in my markdown.

  .gatsby-resp-image-background-image {
    display: none !important;
  }

Otherwise I will have a large white space before my image gets rendered.

Screenshot 2019-07-07 at 18 14 18

My images are used with the normal format in my markdown: ![my image alt](./images/my-image.jpg)


Dependencies:

  "dependencies": {
    "@mdx-js/mdx": "^1.0.21",
    "@mdx-js/react": "^1.0.21",
    "axios": "^0.19.0",
    "disqus-react": "^1.0.6",
    "dotenv": "^8.0.0",
    "gatsby": "^2.13.6",
    "gatsby-image": "^2.2.4",
    "gatsby-link": "^2.2.0",
    "gatsby-mdx": "^0.6.3",
    "gatsby-plugin-catch-links": "^2.1.0",
    "gatsby-plugin-feed": "^2.3.2",
    "gatsby-plugin-google-analytics": "^2.1.1",
    "gatsby-plugin-manifest": "^2.2.1",
    "gatsby-plugin-offline": "^2.2.1",
    "gatsby-plugin-react-helmet": "^3.1.0",
    "gatsby-plugin-sentry": "^1.0.1",
    "gatsby-plugin-sharp": "^2.2.3",
    "gatsby-plugin-sitemap": "^2.2.1",
    "gatsby-plugin-styled-components": "^3.1.0",
    "gatsby-remark-autolink-headers": "^2.1.0",
    "gatsby-remark-copy-linked-files": "^2.1.0",
    "gatsby-remark-images": "^3.1.3",
    "gatsby-source-filesystem": "^2.1.2",
    "gatsby-transformer-remark": "^2.6.1",
    "gatsby-transformer-sharp": "^2.2.1",
    "prism-react-renderer": "^0.1.7",
    "prismjs": "^1.16.0",
    "react": "next",
    "react-dom": "next",
    "react-facebook-pixel": "^0.1.3",
    "react-helmet": "~5.2.1",
    "react-quora-pixel": "0.0.5",
    "react-twitter-conversion-tracker": "^1.0.0",
    "react-youtube": "^7.9.0",
    "styled-components": "^4.3.2"
  },
@rwieruch
Copy link
Contributor Author

rwieruch commented Jul 7, 2019

Also I need to set

  .gatsby-resp-image-image {
    max-width: 100% !important;
  }

to not overflow the image to the right (see screenshot from first comment). But that's what I had already to do before. So no change here.

@cwgw
Copy link
Contributor

cwgw commented Jul 7, 2019

I'm seeing similar layout problems in my own site. Also using gatsby-mdx and gatsby-remark-images.

I think the problem is the inline style attribute on the img element (.gatsby-resp-image-image). It's gone now.

Looks like some hard-coded inline styles were removed by c4a7c40. See here.

I'm not sure if this was intended or not. @JoshuaWalsh @wardpeet could you chime in? Should we now be adding these styles ourselves or is something going wrong here?

@cwgw
Copy link
Contributor

cwgw commented Jul 7, 2019

@rwieruch if you add the styles back, I think things will look they way they did before.

.gatsby-resp-image-image {
  width: 100%;
  height: 100%;
  margin: 0;
  vertical-align: middle;
  position: absolute;
  top: 0;
  left: 0;
}

The problem isn't with .gatsby-resp-image-background-image, it's that the img after it should be absolutely positioned and it no longer is.

@cwgw
Copy link
Contributor

cwgw commented Jul 8, 2019

Looking at c4a7c40 more carefully, it looks like gatsby-remark-images is supposed to add the .gatsby-resp-image-image styles back in via a style tag in the head.

This doesn't seem to be happening for me in development or production. Could the problem be a conflict with gatsby-plugin-mdx?

@YoshiWalsh
Copy link
Contributor

it looks like gatsby-remark-images is supposed to add the .gatsby-resp-image-image styles back in via a style tag in the head

Yep, it's supposed to. The commit message mentions the rationale. I'm sorry to hear this isn't working for you, I will try to help find a resolution.

Could the problem be a conflict with gatsby-plugin-mdx?

Possibly, I haven't used gatsby-plugin-mdx myself. I had a quick look and it doesn't seem to use the replaceRenderer API, so I'm not sure why the head component I added wouldn't be rendered.

Is there a starter or public repository that reproduces the problem?

@cwgw
Copy link
Contributor

cwgw commented Jul 8, 2019

Thank you for your help. I'll try to isolate the issue with a repo.

@chasemccoy
Copy link
Contributor

I can confirm that this happens to my site when updating gatsby-remark-images from 3.1.2 to 3.1.3.

@cwgw
Copy link
Contributor

cwgw commented Jul 8, 2019

Okay, I've tracked it down. Here's a repo.

I think this is a problem with gatsby-plugin-mdx, not gatsby-remark-images, and c4a7c40 just made it visible.

My guess is that gatsby-plugin-mdx isn't properly loading/running the gatsby api calls made by gatsby-remark-images.

If you configure both gatsby-plugin-mdx and gatsby-transformer-remark to use gatsby-remark-images as a plugin, then everything works as expected.

    ...
    {
      resolve: `gatsby-transformer-remark`,
      options: {
        plugins: [ `gatsby-remark-images` ],
      }
    },
    {
      resolve: 'gatsby-plugin-mdx',
      options: {
        gatsbyRemarkPlugins: [ `gatsby-remark-images` ],
      }
    },

If you don't use gatsby-remark-images as a gatsby-transformer-remark plugin, or don't use gatsby-transformer-remark at all, then we see our issue.

    ...
    {
      resolve: `gatsby-transformer-remark`,
      // options: {
      //   plugins: [ `gatsby-remark-images` ],
      // }
    },
    {
      resolve: 'gatsby-plugin-mdx',
      options: {
        gatsbyRemarkPlugins: [ `gatsby-remark-images` ],
      }
    },

@vpicone
Copy link
Contributor

vpicone commented Jul 10, 2019

@ChristopherBiscardi do you think could be related to gatsby-plugin-mdx? It’s happening in our theme when we try and upgrade plugins as well.

@spences10
Copy link
Contributor

I'm having this issue too but run my transformer plugins through gatsbyRemarkPlugins and don't use transformer remark.

Closed #15541 in favour of this.

@cwgw
Copy link
Contributor

cwgw commented Jul 10, 2019

So digging a little deeper, it looks like Gatsby only looks for "subplugins" at one specific path, options.plugins.

gatsby-plugin-mdx uses options.gatsbyRemarkPlugins. This is fine for transforming markdown as the plugin handles that itself, but Gatsby-specific api files like gatsby-browser.js don't get loaded because Gatsby doesn't know they exist.

If you try this…

    {
      resolve: 'gatsby-plugin-mdx',
      options: {
        gatsbyRemarkPlugins: [ `gatsby-remark-images` ],
        plugins: [ `gatsby-remark-images` ],
      }
    },

…everything works as it should.

@SilverFox70
Copy link

I am experiencing this same issue. Is the only solution to us cwgw's snippet?

@cwgw
Copy link
Contributor

cwgw commented Jul 10, 2019

I think the solution is to ensure that gatsby-plugin-mdx is loading plugins properly. However, there's a lot going on in the plugin and much of it is over my head.

@ChristopherBiscardi, am I understanding the problem correctly? Is it reasonable to just rename gatsbyRemarkPlugins as plugins?

@LekoArts LekoArts added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby labels Jul 13, 2019
@Divine1
Copy link

Divine1 commented Jul 14, 2019

@cwgw your solution fixed the issue.

@ChristopherBiscardi
Copy link
Contributor

@cwgw Thanks for narrowing this down (and to everyone else who helped). Gatsby's handling of sub-plugins has always been special but I suppose it matters more now than it used to with the recent gatsby-remark-plugin release.

The flattenedPlugins list that gets plugged into the redux store won't have the gatsbyRemarkPlugins from the gatsby-plugin-mdx options because support for sub-plugins using the plugins API key is hard-coded into the load-plugins function (

// Create a "flattened" array of plugins with all subplugins
// brought to the top-level. This simplifies running gatsby-* files
// for subplugins.
const flattenPlugins = plugins => {
const flattened = []
const extractPlugins = plugin => {
plugin.pluginOptions.plugins.forEach(subPlugin => {
flattened.push(subPlugin)
extractPlugins(subPlugin)
})
}
plugins.forEach(plugin => {
flattened.push(plugin)
extractPlugins(plugin)
})
return flattened
}
), which is why when the api-runner-ssr goes to execute the gatsby-ssr functions from that earlier commit (onRenderBody), it doesn't find it unless the plugin is also specified in plugins. gatsby-plugin-mdx technically uses the plugins array for a prototype shortcodes and components sub-plugin supply implementation in it's wrapRootElement, but gatsby-remark-images is a no-op for the files gatsby-plugin-mdx uses for that so the double-inclusion happens to work in the gatsby-config.

SO, I'm not sure what the best option is here. Adding support to load-plugins to load alternative keys like gatsbyRemarkPlugins seems like unnecessary complexity since gatsby-plugin-mdx is the only plugin that I'm aware of that loads additional plugins like this. So additional complexity in core for one use case is a bit meh.

The other option is to move gatsbyRemarkPlugins to operate out of plugins. This has the downside of being a technically breaking change (even though it can be done backward compatibly) and means we can't use the plugins array for gatsby-mdx specific plugins (because we do some API smoothing over to run gatsbyRemark plugins as remark plugins and that turns out to be a bit complicated where we need to do things like pass in pieces of the node system and such).

I think something that could have a large impact on this might be to work the gatsby-remark-* plugin API surface back into compatibility with the regular remark plugins, which would also have the benefit of unifying the ecosystem again. That would make our gatsbyRemarkPlugin compat support layer unnecessary and we could entertain overlaying the remark plugin... but I haven't had a chance to work deeply on this yet, it is a big chunk of work, and will also possibly not totally solve the problem here.


I wonder what @gatsbyjs/core thinks about enabling arbitrarily keyed sub-plugins (see below for pseudo-code where a, b, and c would contain gatsby-ssr, etc files) to participate in the gatsby-* runners.

module.exports = {
  plugins: [{
    resolve: `gatsby-plugin-mdx`,
    options: {
      subPlugins: [`a`, `b`, `c`]
    }
  }]
}

@cwgw
Copy link
Contributor

cwgw commented Jul 18, 2019

@ChristopherBiscardi thank you for the detailed explanation. And thank you for all of the work on MDX in Gatsby!

You've made the point a few times that using MDX obviates the need for a bunch of the gatsby-remark plugins.

I'm kind of in over my head here, but I think a fourth option would be to just drop support for gatsby-remark-* plugins.

I've looked at your autolink-header and prism replacements at gatsby-mdx. It looks like they just inject components. As you envision them, do gatsby-mdx-* plugins ever touch the AST?

What if gatsby-mdx-* plugins optionally exported transformRemark and transformRehype functions which could be added to the transformation pipeline? You could do a transformation and inject a component in the same plugin.


All I really want out of gatsby-remark-images is to turn something like this

![my image](./images/my-image.jpg)

into something like this

<GatsbyImage fluid={/* image data generated from my-image.jpg */} />

If we could use variables in static queries then I wouldn't need a plugin. I could just write my own component around gatsby-image and import it directly in my mdx files.

In lieu of that, I could do what gatsby-remark-images does, but instead of inserting hardcoded html I could insert jsx and map it to GatsbyImage.

I wrote a simple plugin that does this and loaded it as a vanilla gatsbyRemark plugin, then included GatsbyImage as a component in MDXProvider. It's naive but it gets the job done.

If gatsby-mdx-* plugins could touch the AST then something like this could be bundled into a nice replacement for gatsby-remark-images.

glennreyes added a commit to glennreyes/glennreyes.com that referenced this issue Jul 19, 2019
pakonda added a commit to adblock-thai/site that referenced this issue Jul 20, 2019
@KyleAMathews
Copy link
Contributor

I wonder what @gatsbyjs/core thinks about enabling arbitrarily keyed sub-plugins (see below for pseudo-code where a, b, and c would contain gatsby-ssr, etc files) to participate in the gatsby-* runners.

I think this is reasonable. There wasn't any particular reason to hard-code plugins other than that's what made sense for gatsby-transformer-remark when I was writing this code.

The goal is to enable plugins to create their own sub-plugin ecosystem that works however makes sense for it while still letting these sub-plugins take advantage of Gatsby APIs.

@ChristopherBiscardi
Copy link
Contributor

You've made the point a few times that using MDX obviates the need for a bunch of the gatsby-remark plugins.

I'm kind of in over my head here, but I think a fourth option would be to just drop support for gatsby-remark-* plugins.

We can't deprecate support because that means migration for people using gatsby-transformer-remark becomes harder. We still want switching costs from remark to mdx to be very low and people to replace their gatsby-remark-* plugins at a pace and time that works for them.

Also gatsby-remark-images is one of the few plugins that gatsby-mdx can not replicate without StaticQuery variables, which will come eventually but don't currently exist.

I've looked at your autolink-header and prism replacements at gatsby-mdx. It looks like they just inject components. As you envision them, do gatsby-mdx-* plugins ever touch the AST?

We could likely allow gatsby-mdx-* plugins to add plugins to the remarkPlugins and rehypePlugins lists. This is a bit farther out since we have to solve the current gatsbyRemark backward compat support first. (the gatsby-mdx plugins were, and still are a prototype that is dark-shipped, so there's more work to do).

@KyleAMathews

The goal is to enable plugins to create their own sub-plugin ecosystem that works however makes sense for it while still letting these sub-plugins take advantage of Gatsby APIs.

cc/ @sidharthachatterjee with whom I was discussing this the other day. Should I create a new issue to track this? Seems like we have a couple options for how to do it. If we enable this, then we would also be able to enable remark-* plugins to use gatsby apis, getting us one step closer to having a solution where gatsby-remark-* plugins can be transparently used in the same place as remark plugins.

@KyleAMathews
Copy link
Contributor

Should I create a new issue to track this?

👍 yeah let's move discussion on implementing this to a new issue

@raae
Copy link

raae commented Dec 30, 2019

Is this being worked on? Its an issue for queen-raae/gatsby-remark-oembed#66 as well.

@DavidSabine
Copy link

Hi. Can anyone confirm this issue is resolved? It seems my workaround is no longer required.

@karlhorky
Copy link
Contributor

karlhorky commented Jan 22, 2020

@DavidSabine which workaround are you using? The what does your Gatsby configuration look like 1) before (with the workaround) and 2) now (without the workaround)?

@DavidSabine
Copy link

@DavidSabine which workaround are you using? The what does your Gatsby configuration look like 1) before (with the workaround) and 2) now (without the workaround)?

Hi @karlhorky, thanks for the question. My comment was cryptic I suppose.

So: FIRST, note that my gatsby site inherits gatsby-remark-images from gatsby-theme-blog-core. So a couple of the 'workarounds' above (in gatsby-config.js) I've avoided.

I used styles to workaround the problem.

span.gatsby-resp-image-background-image {
    padding-bottom: 0 !important;
}

That seemed to do the trick.

On January 17th I noticed that all the images in my blog had disappeared. (https://scrum.works/articles) In fact, they had height of zero pixels (and my padding-bottom override was preventing span.gatsby-resp-image-wrapper from expanding to the height of the image. (gatsby-image-remark uses padding-bottom: x% (where x is a ratio of the image's width) to control the height of the container.

Yesterday, I removed my workaround and, voila!, images appear normally now. So, I wonder if others in this thread have observed a major change lately in the output of this plugin.

@sarahbethfederman
Copy link

Possibly related? #20808

@LekoArts LekoArts changed the title [gatsby-remark-images]: gatsby-resp-image-background-image [gatsby-plugin-mdx]: Double images / blurry / wrong style Feb 27, 2020
@lannonbr
Copy link
Contributor

Why was this closed @rwieruch? I don't believe this has been solved yet

@rwieruch
Copy link
Contributor Author

Yesterday I updated to the recent Gatsby versions and I was able to remove the workaround code. I think this is fixed.

rbeer added a commit to rbeer/developer.centrifuge.io that referenced this issue Apr 22, 2020
rbeer added a commit to rbeer/developer.centrifuge.io that referenced this issue Apr 22, 2020
@karlhorky
Copy link
Contributor

karlhorky commented Apr 24, 2020

Hm, this seems to still be an active bug, at least when using with gatsby-remark-images-medium-zoom.

I still needed to duplicate the plugin also to the plugins option key - didn't work without it being both in plugins and gatsbyRemarkPlugins:

JaeYeopHan/gatsby-remark-images-medium-zoom#13

@rwieruch maybe consider reopening?

This issue has gotten some visibility as a single point of reference for this bug, and is even referred to in some starters:

https://github.com/hagnerd/gatsby-starter-blog-mdx/blob/fb1966a296bbfd9f5f06e2d065705b97f1cb123b/gatsby-config.js#L30-L31

jannikbuschke added a commit to jannikbuschke/gatsby-antd-docs that referenced this issue May 11, 2020
workaround for gatsbyjs/gatsby#15486
and also this might be useful in general?
@manoellribeiro
Copy link

I was having this issue, and I solved by deleting "gatsby-resp-image-background-image" and "gatsby-resp-image-image" css styles that I was using.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests