Skip to content
This repository was archived by the owner on Apr 6, 2025. It is now read-only.

Allow using Link without Hyperapp #32

Closed
wants to merge 34 commits into from
Closed

Allow using Link without Hyperapp #32

wants to merge 34 commits into from

Conversation

vdsabev
Copy link

@vdsabev vdsabev commented Dec 21, 2017

Why is this useful?

I'm using the router with picodom instead of Hyperapp with great success - it does outstanding work for such a small router library, and plays well with other architectures. The only component that doesn't work out of the box is Link, because it imports h from Hyperapp.

What's the workaround?

I've basically copied all of the code from Link.js into my own application, with the only difference being I'm importing a different h function - the one from picodom.

Is there a better way?

If Link could somehow receive h as a parameter, it would completely decouple it from Hyperapp and allow everyone to use it however they want.

What did I do?

I moved most of the code from Link.js to a new script called createLink.js. It exports a function with a single parameter - h, which returns what used to be in Link.js. So we can now do this in Link.js instead:

import { h } from "hyperapp"
import { createLink } from "./createLink"

export var Link = createLink(h)

At the same time, in my own application, I can use picodom with the same effort:

import { h } from 'picodom';
import { createLink } from '@hyperapp/router/src/createLink';

export const Link = createLink(h);

What about other libraries?

There's really nothing limiting this to picodom. Any other VDOM library should work great, and I don't see why you can't use that without VDOM, probably even with vanilla JS, by passing a custom render function:

export const Link = createLink((tag, props, children) => {
  const el = document.createElement(tag);
  for (let key in props) {
    el[key] = props[key];
  }
  // append the children somehow...out of scope for this PR 😋

  return el;
});

Jorge Bucaran and others added 30 commits June 30, 2017 03:36
To import the router use:

import { Router } from "@hyperapp/router"

or

const { Router } = require("@hyperapp/router")

This opens the door to adding future exports like <Link />, etc.
- No breaking changes.
- Add new .index property to route info object.
- Introduce Link component. Use it to create hyperlinks
  that map to a particular route.

    <Link to="/" go={actions.router.go}>Home</Link>

  Link is a facade around <a href={to} onclick={()=>go(to)}/>
  that calls element.preventDefault() for you to prevent page
  reloads.

- Add live example http://hyperapp-router.surge.sh and update
  documentation.

- Add tests.
Click handler should only prevent default behavior if left-click/enter key is used and no modifier keys are pressed.  This is common among most other libraries.

- Link target handling & refactor

- Link - added brackets around early return

- Link - change 'e' -> 'event'

- Remove fake preventDefault() from some test cases

- Link - check if same origin
For example:

  Link({to: '/', go: actions.router.go },[
    h('img', {class: 'logo', src: '/static/logo.svg'})
  ])

In this code, when you click the image link, the event.target is actually the
img tag, since you clicked it. event.currentTarget should be used instead, 
since it always references the element which has the event listener.
- location module
- Route
- Redirect
- Link
- Switch
@codecov
Copy link

codecov bot commented Dec 21, 2017

Codecov Report

Merging #32 into master will increase coverage by 2.26%.
The diff coverage is 18.18%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #32      +/-   ##
=======================================
+ Coverage   37.73%   40%   +2.26%     
=======================================
  Files           6     7       +1     
  Lines          53    55       +2     
  Branches       12    12              
=======================================
+ Hits           20    22       +2     
  Misses         24    24              
  Partials        9     9
Impacted Files Coverage Δ
src/createLink.js 10% <10%> (ø)
src/Link.js 100% <100%> (+100%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4949a31...2f73228. Read the comment docs.

@vdsabev
Copy link
Author

vdsabev commented Mar 30, 2018

@jorgebucaran can we reconsider this PR? The solution I described is still valid as of 0.5.1. It would still be useful to reduce the dependency on hyperapp and avoid copy/pasting code to use Link without relying on h, as you have in 48d81a0.

Let me know what you think and I can redo the PR later today 🤠

@jorgebucaran
Copy link
Owner

jorgebucaran commented Mar 30, 2018

@vdsabev If it isn't too much trouble, please make a new PR so we can review it again! :)

@vdsabev
Copy link
Author

vdsabev commented Mar 30, 2018

Looks like both #36 and #61 are open and contain changes to Link.js. In the interest of avoiding conflicts, should I wait for those to get merged or closed?

@jorgebucaran
Copy link
Owner

@vdsabev You could wait, or send your PR and update/resolve conflicts as those other two get resolved. What do you think? It's up to you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants