Youtube embed placed at top of window

Question

Youtube embed placed at top of window

Closed this issue 5 years ago · 25 comments

As seen on https://5e345ce46cdafd0009b0eb8d--gatsby-theme-nicky-blog.netlify.com/mdx-with-embeds

PR with relevant changes
NickyMeuleman/gatsby-theme-nicky-blog#3
Didn't notice it before, did after.

I upgraded from theme-ui v0.2 to v0.3. That resulted in me deleting my own MDXProvider and moving to shadowing the components file from gatsby-plugin-theme-ui

Answer 1 · 2020-01-31T18:56:31.000Z

Thanks @NickyMeuleman could you also let me know what OS and Browser you’re using?

Answer 2 · 2020-01-31T19:03:23.000Z

OS:

Windows 10

Browser:

Chrome Version 79.0.3945.130 (Official Build) (64-bit)
Firefox 73.0b12 (64-bit)
Microsoft Edge 44.19041.1.0

Developing on WSL2.
Output from gatsby info

  System:
    OS: Linux 4.19 Ubuntu 18.04.3 LTS (Bionic Beaver)
    CPU: (4) x64 Intel(R) Core(TM) i5-2500K CPU @ 3.30GHz
    Shell: 5.4.2 - /usr/bin/zsh
  Binaries:
    Node: 12.4.0 - ~/.nvm/versions/node/v12.4.0/bin/node
    Yarn: 1.17.3 - /usr/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v12.4.0/bin/npm
  Languages:
    Python: 2.7.15+ - /usr/bin/python
  npmPackages:
    gatsby-cypress: ^0.2.8 => 0.2.21

Answer 3 · 2020-01-31T19:04:16.000Z

Thanks @NickyMeuleman 👍

Answer 4 · 2020-01-31T19:08:00.000Z

@NickyMeuleman and one more thing. Can you tell me what other Gatsby plugins you’re using. I had a similar issue recently when this plugin was used with gatsby-plugin-feed

Answer 5 · 2020-01-31T19:11:05.000Z

It's quite the list.
The theme is opensourced, please code-dive!
https://github.com/NickyMeuleman/gatsby-theme-nicky-blog

Answer 6 · 2020-01-31T19:42:40.000Z

Tried installing @mdx-js/react again.
Since the gatsby-mdx-embed docs require it https://gatsby-mdx-embed.netlify.com/#install-peer-dependencies.
Locally, it seemed to work. Published, it doesn't.

Theme-ui migration guide https://theme-ui.com/migrating

@emotion/core and @mdx-js/react are now dependencies of theme-ui and should not be installed separately. If you'd like to use a particular version of each library, use Yarn resolutions.

Is that "should" meant as a forbidding "don't do this"
or as a "it's no longer necessary, but you can if you want to"?
//cc @jxnblk what would be the preferred option here?

edit: hit the "clear cache and rebuild" button on Netlify
Result: https://gatsby-theme-nicky-blog.netlify.com/mdx-with-embeds
So still broken after deploy but not anymore locally.
edit2: Figured out it only breaks when directly visiting the link and not when navigating to it from the site.

Answer 7 · 2020-01-31T22:07:12.000Z

Might it be caused by the gatsby-browsers of gatsby-plugin-theme-ui and the one of this plugin fighting?

This is the code there
https://github.com/system-ui/theme-ui/blob/master/packages/gatsby-plugin-theme-ui/src/provider.js
If I understand correctly the components there are basically the same thing as the components prop the MDXProvider takes.

edit: Worked on this more today and it's pointing towards my hunch being correct, I think?
It happens with gatsby build and gatsby serve but NOT in gatsby dev

Answer 8 · 2020-02-01T16:16:56.000Z

Just pushed a workaround that solves it, but feels like a hack since this plugin is already doing this (it should and somehow it doesn't seem like it's working).
By manually wrapping everything in my layout with the exported MdxEmbedProviderfrom this plugin.
NickyMeuleman/gatsby-theme-nicky-blog@3d3ecd8

This is why I mentioned it feeling wrong, because the plugin is already doing it:
https://github.com/PaulieScanlon/gatsby-mdx-embed/blob/master/%40pauliescanlon/gatsby-mdx-embed/src/gatsby-browser.js#L2

Answer 9 · 2020-02-02T07:09:02.000Z

Just came in to mention that this was happening to me too...

Answer 10 · 2020-02-02T09:30:09.000Z

@NickyMeuleman @ehowey Hey!

yeah this is a bit of tricky one.

The method i'm using in gatsby-ssr, wrapRootElement is supposed to provided .mdx files what they need in order to correctly pass and render the components defined in this plugin.

It seems that when this plugin is used with other plugins that use the same approach, they clash.

I've updated the README with a little code snippet showing how to manually import and use the MdxEmbedProvider Hopefully this will get you out of a fix while i investigate the problem.

Answer 11 · 2020-02-03T11:12:57.000Z

Hey @NickyMeuleman i've created a PR for you #5 I've think i've now fixed the issue and your theme now displays YouTube embeds correctly without the need to manually wrap your content with the MdxEmbedProvider

@ehowey and @spences10 you might also like to upgrade your projects to 0.0.17

This fix should now mean once you've installed the plugin as a dependancy and added it to gatsby-config.js you won't need to do anything else. @spences10 you can now safely remove the manually imported MdxEmbedProvider from your blog post template.

Lemme know if any of you are still experiencing issues.

Cheers!

Answer 12 · 2020-02-03T11:23:06.000Z

Hey @PaulieScanlon I jus got the update come in via renovate and the build is failing:

I'll go ahead and remove the wrapping provider and see what happens. 👍

Answer 13 · 2020-02-03T11:57:58.000Z

@spences10 Hmmm... that's annoying! i wonder if it's something to do with my babel config. and the use of uppercase letters in the file name... hold tight!

Answer 14 · 2020-02-03T12:03:06.000Z

@spences10 ok, can you try upgrading to 0.0.17 maybe something weird is going on with that file. it's named Provider but exports as provider i've renamed it now so maybe that'll fix it?

Answer 15 · 2020-02-03T12:11:18.000Z

@spences10 Oh also, it's worth pointing out that @mdx-js/mdx is now also a peer dependency, in case you don't have that installed but i suspect you do as you're using gatsby-plugin-mdx
README

Answer 16 · 2020-02-03T12:15:46.000Z

Yeah, .17 built and no issues with banner video!

Thanks @PaulieScanlon 🙏

Answer 17 · 2020-02-03T12:25:03.000Z

@spences10 Whey!!! thanks for letting me know! man that was a ball ache! glad it's fixed! might wait for @NickyMeuleman and @ehowey to feedback before i close this issue!

Answer 18 · 2020-02-03T12:26:34.000Z

Confirm working for my situation on Firefox and Edge (Chromium)

Answer 19 · 2020-02-03T13:47:40.000Z

Thanks @PaulieScanlon! Just merged that PR and can confirm the issue is gone.

What do you think caused it? I noticed you changed export syntax for gatsby-srr and gatsby-browser from ESM to CommonJS, could that be related?

Answer 20 · 2020-02-03T13:52:16.000Z

@NickyMeuleman Sweet! thanks for letting me know!

Yeah it's something to do with ES6 and CommonJS module exports in gatsby-ssr.js and perhaps the name of the component i was attempting to use for wrapRootElement.

gatsby-browser.js seems to always be ok with ES6 but gatsby-ssr.js appears to prefer CommonJS 🤷‍♂️ I guess because it runs server side it’s node rather than Babel that’s dealing with the Js?

I've updated my 100DaysOfGatsby blog post about what i changed here

Answer 21 · 2020-02-03T15:45:17.000Z

I'll test it tonight Paul - thanks for all your work on this!

Answer 22 · 2020-02-03T16:05:05.000Z

@ehowey no worries man! Thanks to all you guys for testing it out and finding the bugs! @NickyMeuleman @spences10

Answer 23 · 2020-02-04T03:41:55.000Z

Fixed for me too.

One thing I noticed though - and I don't think this is in your control - is that including the embeds is expensive from a web performance perspective. Went from almost straight 100s with pure markdown to some low 90s with Twitter, and with Youtube it dropped it down to a 60-70. All scores in Lighthouse. Gatsby Cloud makes it easy to see this in real time.

Answer 24 · 2020-02-04T14:13:08.000Z

I recently asked 4lpine (mdx creator) a question about MDXProvider Here is a quote from him, talking about shortcodes in MDX:

Though, it's important to note that they will be added to the default

page bundle (right now, but we'll better optimize later), so you don't want any heavyweight shortcodes. I imagine the bulk of that performance loss is directly related to the iframes those embeds add and everything that's going on there though. For YouTube, Paul Irish made https://github.com/paulirish/lite-youtube-embed. Not sure how different it is, but that might be something interesting to look at. Op di 4 feb. 2020 om 04:41 schreef Eric Howey <notifications@github.com>:

…

Fixed for me too. One thing I noticed though - and I don't think this is in your control - is that including the embeds is expensive from a web performance perspective. Went from almost straight 100s with pure markdown to some low 90s with Twitter, and with Youtube it dropped it down to a 60-70. All scores in Lighthouse. Gatsby Cloud makes it easy to see this in real time. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11?email_source=notifications&email_token=AHGIBBLLTDG6RGDFUJB4X63RBDPYLA5CNFSM4KOL6CQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKWHWDI#issuecomment-581729037>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHGIBBMN6SEV57IIXMH2Y3LRBDPYLANCNFSM4KOL6CQA> .

Answer 25 · 2020-02-04T14:55:13.000Z

@NickyMeuleman @ehowey thanks for your feedback. Yes, i suspect those provider shortcodes are the cause, and yes, it's slightly out of my control, those provide shortcodes kinda are what they are. I'll look into the lite-youtube-embed though that is at least a step in the right direction. I think that resolves this pesky bug now so i'll close this issue but please do open issues if you find anything else not working! thanks team!