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

Add hash to the location prop in @reach/router #7107

Closed
lipis opened this issue Aug 7, 2018 · 20 comments
Closed

Add hash to the location prop in @reach/router #7107

lipis opened this issue Aug 7, 2018 · 20 comments
Labels
stale? Issue that may be closed soon due to the original author not responding any more. status: awaiting author response Additional information has been requested from the author type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change

Comments

@lipis
Copy link
Contributor

lipis commented Aug 7, 2018

While I have access to it during develop I'm getting an error during build:


  26 |   getHash() {
> 27 |     return parseInt(this.props.location.hash.slice(1)) || LIMIT;
  28 |   }
  29 |


  WebpackError: TypeError: Cannot read property 'slice' of undefined
@KyleAMathews
Copy link
Contributor

Yes, we switched to @reach/router. Please check out the migration guide! Would love your feedback on it if there's anything missing or where we can help make it better. https://next.gatsbyjs.org/docs/migrating-from-v1-to-v2/#migrate-from-react-router-to-reachrouter

@KyleAMathews
Copy link
Contributor

That being said, I'm not sure why it'd work in development and not in production.

@lipis
Copy link
Contributor Author

lipis commented Aug 7, 2018

@KyleAMathews Actually without making any change the website became very very slow when navigating.. :( Am I holding it wrong?

@Chuloo
Copy link
Contributor

Chuloo commented Aug 7, 2018

@lipis can you share a simple reproduction repo showing your error? or precise steps to reproduce it.

@Chuloo Chuloo added the status: needs more info Needs triaging and reproducible examples or more information to be resolved label Aug 7, 2018
@KyleAMathews
Copy link
Contributor

Known issue on slowness #7068

But yeah, would love to know more about where/how you're using location prop.

@lipis
Copy link
Contributor Author

lipis commented Aug 7, 2018

@Chuloo I will try to if I have time today.

@lipis
Copy link
Contributor Author

lipis commented Aug 7, 2018

Here is the minimal example of the said error and how it's used in the actual project:

import React from 'react'
import Layout from '../components/layout'

class IndexPage extends React.Component {
  constructor(props) {
    super(props)
    this.state = {
      limit: this.getHash(),
    }
  }

  getHash() {
    return parseInt(this.props.location.hash.slice(1)) || 4
  }

  render() {
    console.log('location', this.props.location)
    return (
      <Layout>
        <h1>Limit {this.state.limit}</h1>
      </Layout>
    )
  }
}

export default IndexPage

You can checkout the whole project as is https://github.com/lipis/gatsby-location

Works as expected during develop but throwing an error during build.

 11 |
  12 |   getHash() {
> 13 |     return parseInt(this.props.location.hash.slice(1)) || 4
     |                                              ^
  14 |   }
  15 |
  16 |   render() {


  WebpackError: TypeError: Cannot read property 'slice' of undefined

@KyleAMathews
Copy link
Contributor

Oh yeah — it seems @reach/router doesn't set a hash on the location object automatically, at least when server rendering. @ryanflorence is that something that'd be reasonable to add to @reach/router?

@KyleAMathews
Copy link
Contributor

(also @lipis thanks for the nice reproduction site 💯)

@millette
Copy link
Contributor

millette commented Aug 7, 2018

Ah, that's probably why my react-intl based site got borked when deployed with a pathPrefix. Working on fixing it, glad I found this issue in migration guide.

UPDATE: The problem (on my side) appeared before beta 71 but I hadn't noticed. I only test deploy with a path prefix to GitHub Pages.

@lipis
Copy link
Contributor Author

lipis commented Aug 9, 2018

@KyleAMathews @ryanflorence What's the status with the location.hash.. we are stuck in the older versions because of that.. :/ and there are daily updates.. we don't want to be far behind.

@KyleAMathews
Copy link
Contributor

@lipis you can check if location.hash exists before trying to access. Will see about adding it to @reach/router as well.

@lipis
Copy link
Contributor Author

lipis commented Aug 9, 2018

Should I change the title accordingly for this issue?

@KyleAMathews
Copy link
Contributor

That'd be great

@lipis lipis changed the title Did something change in the location prop in the latest build 2.0.0-beta.76? Add hash to the location prop in @reach/router Aug 9, 2018
@lipis
Copy link
Contributor Author

lipis commented Aug 9, 2018

Updated.. feel free to edit it as well if it doesn't make sense :)

@Chuloo Chuloo added type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change and removed status: needs more info Needs triaging and reproducible examples or more information to be resolved labels Aug 22, 2018
@gatsbot
Copy link

gatsbot bot commented Dec 26, 2018

Old issues will be closed after 30 days of inactivity. This issue has been quiet for 20 days and is being marked as stale. Reply here or add the label "not stale" to keep this issue open!

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Dec 26, 2018
@lipis
Copy link
Contributor Author

lipis commented Dec 27, 2018

Are we doing this? Would be nice.

@lipis lipis added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Dec 27, 2018
@wardpeet
Copy link
Contributor

@lipis it should work now as reach router added search & hash props to SSR part of reach router. Could you give it another spin?

https://github.com/reach/router/blob/b60e6dd781d5d3a4bdaaf4de665649c0f6a7e78d/CHANGELOG.md#v120

@wardpeet wardpeet added status: awaiting author response Additional information has been requested from the author and removed not stale labels Feb 20, 2019
@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Mar 13, 2019
@gatsbot
Copy link

gatsbot bot commented Mar 13, 2019

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

@gatsbot
Copy link

gatsbot bot commented Mar 24, 2019

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

Thanks again for being part of the Gatsby community!

@gatsbot gatsbot bot closed this as completed Mar 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale? Issue that may be closed soon due to the original author not responding any more. status: awaiting author response Additional information has been requested from the author type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

No branches or pull requests

5 participants