fridays / next-routes

Universal dynamic routes for Next.js
MIT License
2.47k stars 230 forks source link

Feature/support anchor hashes #91

Closed zentuit closed 1 year ago

zentuit commented 7 years ago

The Link component should handle getting an anchor hash parameter and creating the correct href and as values.

If the user is passing in href then ignore the hash parameter as the user can build the proper url object themselves. We just make sure it gets passed to next's Link correctly.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-3.8%) to 96.186% when pulling 48c987025c789c3216ae235d47323755e60241db on zentuit:feature/support_hashes into 7548fcbcf2cb02d7696ab64091eae5d54b1b947d on fridays:master.

zentuit commented 7 years ago

I think I need to make a corresponding update to Router.pushRoute

zentuit commented 7 years ago

I don't think this is necessary with the ability to provide own Link/Router

jaydenseric commented 6 years ago

I'm not following why this is not necessary; what is the right way to add a hash to a named route in JSX?

zentuit commented 6 years ago

@jaydenseric : this worked:

const React = require('react')
const PropTypes = require('prop-types')
const NextLink = require('next/link').default
const NextRouter = require('next/router').default

const addHash = (href, as, hash) => (
  hash === undefined ? { href, as } : { href: `${href}#${hash}`, as: `${as}#${hash}`, }
)

const HashAwareRouter = ['push', 'replace', 'prefetch'].reduce((result, key) => {
  // eslint-disable-next-line no-param-reassign
  result[key] = (href, as = href, rest = {}) => {
    const updatedPaths = addHash(href, as, rest.hash)
    return NextRouter[key](updatedPaths.href, updatedPaths.as, rest)
  }
  // eslint-disable-next-line no-param-reassign
  result.nextRouter = NextRouter
  return result
}, {})

const HashAwareLink = (props) => {
  const newProps = Object.assign({}, props, addHash(props.href, props.as, props.hash))
  delete newProps.hash
  return React.createElement(NextLink, newProps)
}

HashAwareLink.propTypes = {
  href: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),
  as: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),
  hash: PropTypes.string,
}

const routes = require('next-routes')({
  Link: HashAwareLink,
  Router: HashAwareRouter,
})

module.exports = routes

routes.add(.......)
jaydenseric commented 6 years ago

Thanks for sharing all of that 🙂

But I think your PR is way nicer, maybe it should be reopened?

It makes sense that hash can be used on a named route, just the same as params.

trotzig commented 6 years ago

I'm in favor of getting this PR re-opened as well. It seems clunky for everyone to have to define their own Link component in order to support #hashes in Link urls.

ckeeney commented 6 years ago

What is the status of this PR? I believe merging this PR would close #232 and #165.

revolunet commented 5 years ago

Hi, if anyone could try #258 that includes this PR and adds pushRoute support, would be nice. thanks.

rg-najera commented 5 years ago

@revolunet I think this is sitting here because the coverage decreased significantly.

osym commented 5 years ago

Hey,

Any news here?