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

[1.0] render component only in browser #931

Closed
j000i opened this issue May 8, 2017 · 39 comments · May be fixed by harunpehlivan/gatsby#182 or philipedin/gatsby#195
Closed

[1.0] render component only in browser #931

j000i opened this issue May 8, 2017 · 39 comments · May be fixed by harunpehlivan/gatsby#182 or philipedin/gatsby#195

Comments

@j000i
Copy link

j000i commented May 8, 2017

Hello! Great work on the latest Gatsby alpha14!

I have a component that fetches headlines via RSS that I only want to render in the client. Is there a recommended way to do this?

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 8, 2017 via email

@j000i
Copy link
Author

j000i commented May 8, 2017

Hello @KyleAMathews! Thanks for the quick response (and all your amazing work)!

I have a something along the lines of the following in my index.js:

import Feed from "../components/Feed"

class Index extends React.Component {
  constructor() {
    super()

    this.state = {
      renderFeed: false,
    }
  }

  componentDidMount() {
    this.setState({ renderFeed: true })
  }

  render() {
    const { renderFeed } = this.state

    return (
      <div>
        {!renderFeed ? null : <Feed />}
      </div>
    )
  }
}

This works great in development mode (gatsby develop) – the component renders, but when I do gatsby build && gatsby serve-build, the component does not render I get an error in the browser console:

AsyncUtils.js:79 Uncaught TypeError: n is not a function
    at o (AsyncUtils.js:79)
    at AsyncUtils.js:85
    at split-child-routes.js:17
    at split-child-routes.js:18
    at Function.t.e (bootstrap 15f086a…:66)
    at split-child-routes.js:15
    at Function.t.e (bootstrap 15f086a…:66)
    at Object.getComponent (split-child-routes.js:13)
    at r (getComponents.js:29)
    at getComponents.js:41

Any clue what this could be? Not sure what information I can provide to help you help me. I think I tried something like this when I played with alpha12 and it worked, but I trashed the source … :-/

@j000i
Copy link
Author

j000i commented May 8, 2017

PS: Just noticed that when visit another route and then go back, the component renders.

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 8, 2017 via email

@j000i
Copy link
Author

j000i commented May 8, 2017

It's react-routers webpack:///./~/react-router/lib/AsyncUtils.js. Baffled at what n is…

@j000i
Copy link
Author

j000i commented May 8, 2017

An excerpt of react-router/lib/AsyncUtils.js like shown in Chrome's Sources panel. I put a comment above the line where the TypeError occurs:

function mapAsync(array, work, callback) {
  var length = array.length;
  var values = [];

  if (length === 0) return callback(null, values);

  var isDone = false,
      doneCount = 0;

  function done(index, error, value) {
    if (isDone) return;

    if (error) {
      isDone = true;
      callback(error);
    } else {
      values[index] = value;

      isDone = ++doneCount === length;

      // next line is where "n is not a function" occurs
      if (isDone) callback(null, values);
    }
  }

  array.forEach(function (item, index) {
    work(item, index, function (error, value) {
      done(index, error, value);
    });
  });
}

@KyleAMathews
Copy link
Contributor

Try returning an empty <div /> instead of null. Perhaps that's causing trouble with React Router.

@j000i
Copy link
Author

j000i commented May 8, 2017

In an effort to at least eliminate all the other stuff that could be wrong on my side, I just cloned your blog (https://github.com/KyleAMathews/blog) and can confirm that the same thing happens if I add the state.renderFeed bits from above and a simple {!renderFeed ? null : <div>FEED</div>}.

… wow! Your answer just came in. Thank you so much – let me give that a shot!

@j000i
Copy link
Author

j000i commented May 8, 2017

With {!renderFeed ? <div>NO FEED</div> : <div>FEED</div>} I now get a "FEED" with gatsby developand a "NO FEED" after build && build-serve, while the error message stays the same. :-(

@KyleAMathews
Copy link
Contributor

BTW, are you exporting the component?

@j000i
Copy link
Author

j000i commented May 8, 2017

My original feed component yes. But the inline {!renderFeed ? <div>NO FEED</div> : <div>FEED</div>} raises the same error.

I just tried the simple NO FEED/FEED "test" with your blog at KyleAMathews/blog@7fa4ded with alpha13 and there things seem to work as expected: The output with JavaScript activated for both develop and build && serve-build is "FEED", and if I disable JavaScript in serve-build, I get a "NO FEED" (without errors).

@KyleAMathews
Copy link
Contributor

Hmmmm seems like you've found a bug of some sort.

Do you want to try using git bisect to find when it was introduced? You could use that plus the instructions here for running Gatsby from a git clone https://www.gatsbyjs.org/docs/how-to-contribute/

@KyleAMathews
Copy link
Contributor

Here's a good tutorial for using git bisect if you haven't used it before http://webchick.net/node/99

@j000i
Copy link
Author

j000i commented May 8, 2017

Of course! Will take a look!

@j000i
Copy link
Author

j000i commented May 8, 2017

Wow, didn't know about git bisect, nice! I will see if I can get my head around stepping through compiling stuff, don't know if I can go along with the changes to the GraphQL layer myself. ;-)

For today I can add one more thing – I found the alpha13 version that I mentioned earlier where things were still working: 1.0.0-alpha13-alpha.537d11c6.

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 8, 2017 via email

@j000i
Copy link
Author

j000i commented May 8, 2017

Just to be in the clear about this – one should do npm install && lerna bootstrap && npm run build after every checkout that bisect does, right?

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 8, 2017 via email

@KyleAMathews
Copy link
Contributor

KyleAMathews commented May 8, 2017 via email

@j000i
Copy link
Author

j000i commented May 8, 2017

Many changes are code only. Those are necessary only if package dependencies change

So only npm run build and gatsby-dev are necessary if only code changed…!? ;-)
Btw., is it correct that I have to quit the gatsby-dev process myself? All files seem to copy correctly.


Going the super-safe npm install && lerna bootstrap && npm run build route after each checkout (but with the question regarding the gatsby-dev force-quit in mind), I think nailed it down to f66abd3 – with that commit, the described TypeError in react-router's AsyncUtils occurs the first time. One commit before, 7b45d92, is still fine.

My pages/index.js that I used to test, in all its glory:

import React from "react"
import PropTypes from "prop-types"

class Index extends React.Component {
  constructor() {
    super()

    this.state = {
      renderFeed: false,
    }
  }

  componentDidMount() {
    this.setState({ renderFeed: true })
  }

  render() {
    const { renderFeed } = this.state
    const author = this.props.data.site.siteMetadata.author

    return (
      <div>
        <h1>
          {author}
        </h1>

        {!renderFeed ? <div>NO FEED</div> : <div>FEED</div>}
      </div>
    )
  }
}

Index.propTypes = {
  data: PropTypes.object.isRequired,
}

export default Index

export const pageQuery = `{
    site {
      siteMetadata {
        author
      }
    }
  }
`

The TypeError also occurs if I remove the pageQuery related stuff:

import React from "react"

class Index extends React.Component {
  constructor() {
    super()

    this.state = {
      renderFeed: false,
    }
  }

  componentDidMount() {
    this.setState({ renderFeed: true })
  }

  render() {
    const { renderFeed } = this.state
    return (
      <div>
        {!renderFeed ? <div>NO FEED</div> : <div>FEED</div>}
      </div>
    )
  }
}

export default Index

@0x80
Copy link
Contributor

0x80 commented May 8, 2017

That commit added an error log to gatsby-link which wasn't there before. This error is about production code only. It could be that the error was there before but got swallowed.

Regarding gatsby-dev, it ends in watch mode. So killing it only means you stop the file watch. You can use the --scan-once option instead.

@j000i
Copy link
Author

j000i commented May 8, 2017

This error is about production code only.

👍

It could be that the error was there before but got swallowed.

While I don't know that, the behavior def. changes with f66abd3 as the simple index.js now renders "NO FEED" in production now instead of "FEED", no matter if JavaScript is activated or not. :-(

@0x80
Copy link
Contributor

0x80 commented May 8, 2017

On second thought since you're not even using gatsby-link, I guess the error handling part is likely not related 🙂 The commit is messy because unrelated commits seemed to have merged into one, but I don't see anything suspicious...

@j000i
Copy link
Author

j000i commented May 8, 2017

The commit is messy because several unrelated commits were merged into one, but I don't see anything suspicious...

I tried to look through it and didn't see anything jumping at me, but I doubt I can be of any help here.
I'm also quite concerned to not put you guys on the wrong hunting path :-/ – I hope I did the right things to isolate the problem and test the Gatsby builds, but I'm not too sure.

@j000i
Copy link
Author

j000i commented May 8, 2017

I had been testing in Chrome only up to now, now I gave Firefox and Safari as well as Safari on iOS a spin: Firefox does not show the error in the console, it just does not render the component at all. Both Safaris raise the same error as Chrome.

@KyleAMathews
Copy link
Contributor

@j000i f66abd3 is the commit you found using git bisect?

@j000i
Copy link
Author

j000i commented May 8, 2017

@KyleAMathews I did do it manually with git checkout, then npm install && lerna bootstrap && npm run build, then copied files over to the test repo with gatsby-dev --scan-once (after setting the path to my local gatsby build). I got a little confused with rebuilding, that's why I didn't use git bisect.

I'm still debugging btw. (well, trying to ;-)) – if I strip all tag- and blog-related code and Helmet, etc., things are working. I'm currently removing stuff one-by-one to find out when things start working.

@j000i
Copy link
Author

j000i commented May 8, 2017

I think I got it! 🎉 ;-)
Sorry for all the noise!

If I would have read more thoroughly, I would have thought a little more about things when @0x80 mentioned the following:

On second thought since you're not even using gatsby-link, I guess the error handling part is likely not related

In fact, html.js was importing gatsby-link to show a link to the homepage. In the process of stripping down things, I removed that <Link>, and things worked. Then I added it back to my index.js, and when it looks like

import React from "react"
import Link from "gatsby-link"

class Index extends React.Component {
  constructor() {
    super()

    this.state = {
      renderFeed: false,
    }
  }

  componentDidMount() {
    this.setState({ renderFeed: true })
  }

  render() {
    const { renderFeed } = this.state
    return (
      <div>
        <Link to="/">
          Home
        </Link>
        {!renderFeed ? <div>NO FEED</div> : <div>FEED</div>}
      </div>
    )
  }
}

export default Index

(which is identical to what I posted in my previous comment apart from adding the <Link>), things go bad. Pheeeww!

@j000i
Copy link
Author

j000i commented May 8, 2017

When I move that link from pages/index.html to html.js, the error does not occur; when it's in layouts/default.js, it does.
Could it be that I'm simply not supposed to use <Link> in components/pages?

@j000i
Copy link
Author

j000i commented May 8, 2017

Last comment (hopefully – I took away too much of your time already): After manually reverting 25dc2f2, rebuilding Gatsby, copying via gatsby-dev --scan-once and gatsby build && gatsby serve-build, things work. So from my understanding it seems @0x80 was very much on point when writing

It could be that the error was there before but got swallowed.

earlier.

@KyleAMathews
Copy link
Contributor

I took away too much of your time already

no not at all! We're actually secretly training you on how to contribute to Gatsby so once you're trained up you'll save us a lot of time by fixing all sorts of things you run into :-) You're doing a really good job working through the problem.

Huh, so yeah, looks like this is a real bug somewhere... I'm actually going to be spending the next few days upgrading Gatsby to use RR v4 so might end up fixing this bug in the process :-)

@j000i
Copy link
Author

j000i commented May 8, 2017

We're actually secretly training you on how to contribute to Gatsby so once you're trained up you'll save us a lot of time by fixing all sorts of things you run into :-)

:) True, I got to know a bunch of new things! 👍

You're doing a really good job working through the problem.

Really? I wouldn't have thought so, thank you!
Glad I am helping!

Huh, so yeah, looks like this is a real bug somewhere... I'm actually going to be spending the next few days upgrading Gatsby to use RR v4 so might end up fixing this bug in the process :-)

Yay for RR4! Good luck!

@fk
Copy link
Contributor

fk commented May 10, 2017

@j000i Chiming in to confirm your findings. I just upgraded a small site I'm building from alpha13 to alpha14, and while things worked fine on localhost, I also ran into the problems your described after deploying the production build and testing.

After patching gatsby-link as per your description everything is "working" again. Thank you for your efforts, and kudos to @KyleAMathews and @0x80 for being so responsive and helpful!

@KyleAMathews
Copy link
Contributor

@fk Love your site btw :-) gorgeous! You should add it to the list of Gatsby sites!

@fk
Copy link
Contributor

fk commented May 10, 2017

@KyleAMathews Thank you so much for your kind words! The site originally started as a small "social links" one pager, but when I saw what you did for https://www.gatsbyjs.org/blog/ regarding (responsive) images, I couldn't help but talking @yisela into adding some of her existing articles.

Things are a little rough around the edges still and we have some more ideas for article layout, but Gatsby + Typography.js made it really, really easy to get off the ground. Btw., that Dribbble feed is based on some old component I had laying around … and guess what, it's the only slow thing on the site. Everything else is blazing fast (and 90% your work – which is much appreciated)! Thank you!

@fk
Copy link
Contributor

fk commented May 12, 2017

Just updated to 1.0.0-alpha14-alpha.ac7022a4 and the issue seems to be resolved! 🎉

@KyleAMathews
Copy link
Contributor

@fk great!

@j000i lemme know if you have a chance to test things. Will hopefully be releasing a alpha15 today (or early next week).

@j000i
Copy link
Author

j000i commented May 12, 2017

Hello @KyleAMathews ... wow that went fast!
I just tested and now it is working! 👍

@j000i j000i closed this as completed May 12, 2017
@KyleAMathews
Copy link
Contributor

Yeah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants