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

Props don't get applied on some divs when using gatsby-plugin-offline #6059

Closed
Maistho opened this issue Jun 20, 2018 · 13 comments
Closed

Props don't get applied on some divs when using gatsby-plugin-offline #6059

Maistho opened this issue Jun 20, 2018 · 13 comments
Labels
help wanted Issue with a clear description that the community can help with. type: question or discussion Issue discussing or asking a question about Gatsby

Comments

@Maistho
Copy link

Maistho commented Jun 20, 2018

Description

Props don't get applied to top level divs when using gatsby-plugin-offline and gatsby-plugin-react-next

The bug manifests on top level divs or divs that are top level inside a Fragment. No props seem to be applied. Other types of elements get props applied as usual.

Steps to reproduce

This repo is based on gatsby-starter-blog:
https://github.com/Maistho/gatsby-offline-react-next-issue/commit/a9e6bbc7d0df4de28707a5a2ca0e0eae2a1785ae

Download the repo and run

gatsby build
gatsby serve

Then open up the browser on a blog post and reload the page. Make sure that service workers are not deactivated.

Expected result

Props should be applied

Actual result

Props are not applied

Environment

Not sure why, but I get some TypeErros when running gatsby info

File contents (if changed)

diff --git a/gatsby-config.js b/gatsby-config.js
index 43a2167..d89ea61 100644
--- a/gatsby-config.js
+++ b/gatsby-config.js
@@ -45,6 +45,7 @@ module.exports = {
       },
     },
     `gatsby-plugin-feed`,
+    `gatsby-plugin-react-next`,
     `gatsby-plugin-offline`,
     `gatsby-plugin-react-helmet`,
     {
diff --git a/package.json b/package.json
index 0a31236..836490d 100644
--- a/package.json
+++ b/package.json
@@ -11,8 +11,9 @@
     "gatsby-link": "^1.6.39",
     "gatsby-plugin-feed": "^1.3.20",
     "gatsby-plugin-google-analytics": "^1.0.29",
-    "gatsby-plugin-offline": "^1.0.15",
+    "gatsby-plugin-offline": "^1.0.18",
     "gatsby-plugin-react-helmet": "^1.0.8",
+    "gatsby-plugin-react-next": "^1.0.11",
     "gatsby-plugin-sharp": "^1.6.42",
     "gatsby-plugin-typography": "^1.7.18",
     "gatsby-remark-copy-linked-files": "^1.5.31",
diff --git a/src/templates/blog-post.js b/src/templates/blog-post.js
index 1231b3d..c0e0f36 100644
--- a/src/templates/blog-post.js
+++ b/src/templates/blog-post.js
@@ -3,6 +3,7 @@ import Helmet from 'react-helmet'
 import Link from 'gatsby-link'
 import get from 'lodash/get'
 
+import styles from './blog-post.module.css'
 import Bio from '../components/Bio'
 import { rhythm, scale } from '../utils/typography'
 
@@ -13,9 +14,22 @@ class BlogPostTemplate extends React.Component {
     const { previous, next } = this.props.pathContext
 
     return (
-      <div>
+      <React.Fragment>
         <Helmet title={`${post.frontmatter.title} | ${siteTitle}`} />
-        <h1>{post.frontmatter.title}</h1>
+        <h1 className={styles.test}>this h1 will get the green class</h1>
+        <div
+          id="test"
+          style={{ background: 'green' }}
+          className={`test ${styles.test}`}
+        >
+          this div will not get any props
+          <div className={styles.test}>But nested divs get props</div>
+          It's a bit weird
+        </div>
+        <section className={styles.test}>
+          But it seems like other tags will get props. This section for example.
+        </section>
+        <h1 className={styles.test}>this h1 will also get the class</h1>
         <p
           style={{
             ...scale(-1 / 5),
@@ -59,7 +73,7 @@ class BlogPostTemplate extends React.Component {
             </li>
           )}
         </ul>
-      </div>
+      </React.Fragment>
     )
   }
 }
diff --git a/src/templates/blog-post.module.css b/src/templates/blog-post.module.css
new file mode 100644
index 0000000..5f18c30
--- /dev/null
+++ b/src/templates/blog-post.module.css
@@ -0,0 +1,3 @@
+.test {
+  background: green;
+}
@m-allanson m-allanson added help wanted Issue with a clear description that the community can help with. type: question or discussion Issue discussing or asking a question about Gatsby labels Jun 22, 2018
@KyleAMathews
Copy link
Contributor

Is it that props aren't applied or is that the css isn't loaded?

@Maistho
Copy link
Author

Maistho commented Jun 28, 2018

Props are not applied at all. It does not get the id or the style either.

@anthkris
Copy link

anthkris commented Jul 7, 2018

I think this might be related to #5136. I noticed that when I uninstall the offline plugin, the issue is cleared up and classNames are applied. Note that className is still visible in React using the React Developer Tools plugin. But after refresh it doesn't show up on the rendered code.

@Maistho
Copy link
Author

Maistho commented Jul 7, 2018

@anthkris yep, seems to be the exact same issue.

@wKovacs64
Copy link
Contributor

I think this might've been fixed in #7355. Seems resolved for me, anyway. Can someone else confirm?

@vtenfys
Copy link
Contributor

vtenfys commented Aug 21, 2018

@wKovacs64 ah yes, I noticed this problem while working on the PR - I think this is actually a bug in React, since as @anthkris said the className is still applied in the VDOM - also, for me the bug only happened in Chrome (or other Blink-based). I worked around it just by changing the offline shell element to a span - it seems to only occur when the page's outer element is the same as the offline shell's outer element (usually div).

image

@vtenfys vtenfys closed this as completed Aug 21, 2018
@anthkris
Copy link

@davidbailey00 I am still on Gatsby V1 and deploying with Netlify. How might I fix this for my website? Is the only option right now to upgrade to V2 in beta?

@Maistho
Copy link
Author

Maistho commented Aug 22, 2018

@davidbailey00 Nice find!

I guess it means that using a top-level <span> in a component would still trigger the bug?

Is there any other element that could be used which would solve this? Perhaps AppShell could render a Fragment instead?

As a side note, though I'm fairly certain that it doesn't actually cause any problems, <span> can only contain phrasing content according to the html spec, so using a div seems more correct in that regard.

@Maistho
Copy link
Author

Maistho commented Aug 22, 2018

I did a quick search on the react issues, and I think it might be related to these issues:

facebook/react#10591

Because the first item is a div in both cases we try to reuse it. That's why it retains its styles. If it was a different tag name it would patch it up but it's risky to rely on this.

facebook/react#12713

@vtenfys
Copy link
Contributor

vtenfys commented Aug 22, 2018

@anthkris

I am still on Gatsby V1 and deploying with Netlify. How might I fix this for my website? Is the only option right now to upgrade to V2 in beta?

I'll backport the fix to V1 since this particular problem is only a one-line change. There might be some other problems in gatsby-plugin-offline which I fixed in the V2 version, but I think on the whole V1 works well (compared to plugin-offline V2 before my PR, which was quite broken).

@Maistho

I guess it means that using a top-level <span> in a component would still trigger the bug?

Theoretically yes - but I haven't tested this

Is there any other element that could be used which would solve this? Perhaps AppShell could render a Fragment instead?

Good idea - having said that, can fragments themselves be empty? I'm fairly new to React so not sure if this would work (but I'll give it a try)

As a side note, though I'm fairly certain that it doesn't actually cause any problems, can only contain phrasing content according to the html spec, so using a div seems more correct in that regard.

The span is replaced with whatever the page's top-level actual element is, as soon as it loads - until then, it's empty, so it's always valid HTML.

@anthkris
Copy link

@davidbailey00 Thanks so much for your help!

@vtenfys
Copy link
Contributor

vtenfys commented Aug 22, 2018

@anthkris Don't mention it! Honestly it's a really easy fix, and @Maistho deserves some credit too for suggesting to use fragments!

@Maistho
Copy link
Author

Maistho commented Aug 22, 2018

Oh it works then? 👌

It was just a very tired suggestion from me, thanks a lot for trying it out and opening PRs @davidbailey00 ♥️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue with a clear description that the community can help with. type: question or discussion Issue discussing or asking a question about Gatsby
Projects
None yet
Development

No branches or pull requests

6 participants