Consider extract the routing code from core #47
Comments
|
@danigb I'm generally in favor of extracting it, but note that this will lead to people using different routing solutions that may not interoperate well with each other. That in itself is not a problem, but the thing I like about |
|
@danigb I've done it in 600 bytes :) last night and it is available at https://github.com/tunnckoCore/gibon and https://unpkg.com/gibon (i'm writing docs now). It is pretty customizable and works well with edit: I'm in process to extract the "diffing" thing, so we can see if we could build |
|
But yea, in anyway it should be in the core. Because the purpose of the |
|
@itrelease Yeah, but this would create a slightly larger bundle. |
|
@jbucaran I think that 1kb shouldn't be a selling point of |
|
@jbucaran I believe @itrelease means to separate helpers into new local file. It won't add additional bytes, because Rollup is smart enough. |
|
@itrelease i'm 👍 here. Also it is not really 1kb, fully functional is ~3.5kb - meh... don't know. |
|
@tunnckoCore What your users will consume is what matters? And what they will consume ~1.6ish kb. If you mean the codepen examples, etc., sure it's 3.1.
Why not? |
|
@jbucaran because at some point I think you would need to add some feature or fix bug that would lead to bigger bundle size but I guess it would be more important than keeping 1kb? |
|
@itrelease Yeah, that's not unlikely. I'll see if rollup is smarter than browserify here as @tunnckoCore said. |
|
@jbucaran it is the father and mother of the tree shaking, it always will be better then the others. As about the size... 1.6kb gzip is for JSX users |
|
@tunnckoCore Yes, and also for Hyperx users alike, if they bundle with hyperxify. |
|
That is one of Rollups big selling points: It merges all of the modules together, as if you had hand-crafted a single file. |
|
@SkaterDad In that case I apologize for my impatience. Browserify didn't do this for me, but rather created small little UMD modules all over the place. Maybe my setup was iffy. |
|
@danigb You still want to get rid of the router? Should we get rid of the router? Please 👍 👎 . |
|
@jbucaran I'm against "getting rid of" it, but making it an optional import would be nice. |
|
Hi, I think would be nice to have it, but I didn't find time to think a
good alternative, so it's ok to me to close this issue 👌
El vie., 10 feb. 2017 16:33, Florian Wendelborn <notifications@github.com>
escribió:
… @jbucaran <https://github.com/jbucaran> I'm against "getting rid of" it,
but making it an optional import would be nice.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAdIbum1716r2JC4_bBMfilbpCjlgkjks5rbINBgaJpZM4L4sEP>
.
|
tl;dr
I've considered this issue again and I'd like to proceed with removing the router from app.js and also from this repository. The current router implementation is "okay" for simple usage, but it's not fair to limit users from using a great router implementation due to HyperApp size goals, etc. I've been fiddling with how the API would look like and I've come up with two approaches. There's actually a third one that looks similar to what @danigb proposed, but it requires app.js to know about pushState and that there is a router, etc. I used the name ARouter.app({
model: 1985,
view: {
"/": (model, msg, params, setLocation) => {
console.log("!!!!", setLocation)
return (
<div>
<h1>Home</h1>
<button onclick={_ => setLocation("/about")}>About</button>
</div>
)
},
"/about": (model, msg, params, setLocation) =>
<div>
<h1>About</h1>
<button onclick={_ => setLocation("/")}>Home</button>
</div>
}
})BRouter.start(app({
model: 1985,
view: {
"/": (model, msg, params, setLocation) => {
console.log("!!!!", setLocation)
return (
<div>
<h1>Home</h1>
<button onclick={_ => setLocation("/about")}>About</button>
</div>
)
},
"/about": (model, msg, params, setLocation) =>
<div>
<h1>About</h1>
<button onclick={_ => setLocation("/")}>Home</button>
</div>
}
}))Capp({
view: Router({
"/": (model, msg, params, setLocation) =>
<div>
<h1>Home</h1>
<button onclick={_ => msg.setLocation("/about")}>About</button>
</div>,
"/about": (model, msg, params, setLocation) =>
<div>
<h1>About</h1>
<button onclick={_ => msg.setLocation("/")}>Home</button>
</div>
})
})/cc @dodekeract |
|
Great! That's good news. I'm against app.js know about pushState. I did a
proof of concept where router used the model to store the current route,
but that enforces to use an object model, and I'm not sure it's a good
opción.
Your A alternative looks good to me. It's important to define a good api
between router and app yo allow different router implementations.
I think the app.js will benefit a lot from this change 👌
El sáb., 11 feb. 2017 7:57, Jorge Bucaran <notifications@github.com>
escribió:
I've been considering this issue again and I'd like to proceed with
removing the router from app.js and from this repository. The current
router implementation is fine for simple usage, but it's not fair to limit
users to use a great router implementation due to HyperApp size goals, etc.
I've been fiddling with how the API would look like and I've come up with
two approaches. There's actually a third one that looks similar to what
@danigb <https://github.com/danigb> proposed, but it requires app.js to
know about pushState and that there could be such a thing as a router.
I used the name Router in the examples, but this is also open to
discussion. Router probably has the best search-ability, but Navigator was
also an eye-candy.
A
Router(app({
model: 1985,
view: {
"/": (model, msg, params, setLocation) => {
console.log("!!!!", setLocation)
return (
<div>
<h1>Home</h1>
<button onclick={_ => setLocation("/about")}>About</button>
</div>
)
},
"/about": (model, msg, params, setLocation) =>
<div>
<h1>About</h1>
<button onclick={_ => setLocation("/")}>Home</button>
</div>
}
}))
B
Router.start(app({
model: 1985,
view: {
"/": (model, msg, params, setLocation) => {
console.log("!!!!", setLocation)
return (
<div>
<h1>Home</h1>
<button onclick={_ => setLocation("/about")}>About</button>
</div>
)
},
"/about": (model, msg, params, setLocation) =>
<div>
<h1>About</h1>
<button onclick={_ => setLocation("/")}>Home</button>
</div>
}
}))
C
app({
view: Router({
"/": (model, msg, params, setLocation) =>
<div>
<h1>Home</h1>
<button onclick={_ => msg.setLocation("/about")}>About</button>
</div>,
"/about": (model, msg, params, setLocation) =>
<div>
<h1>About</h1>
<button onclick={_ => msg.setLocation("/")}>Home</button>
</div>
})
})
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAdIZLnSAVeuT4JutxGKkjOElvYwsqUks5rbVvxgaJpZM4L4sEP>
.
|
* Extract router from app.js.
* Add documentation.
* Have app(...) return { ...options, render(view) {...} } object
which can be used to render a different view.
* Support nodes whose tag is a function. This makes it possible to create jsx components in this way: <MyComponent props=...></MyComponent> * Remove router. Close #47. * Extract router from app.js. * Add documentation. * Have app(...) return { ...options, render(view) {...} } object which can be used to render a different view. * Remove routing tests.
Hi @jbucaran,
I felt in love with
hyperappbecause is almost the perfect essence of the Elm application architecture. All the code is clean, simple and functional 👍 The only part that looks a little bit hacky is the routing code. But there are other reasons to extract it:I don't know the best solution (that's why I'm opening this issue), but I think the API could be something like:
The new
routesfunction should create a view (function) that selects the route proper view function using the browser history, and previously it registers the required event handlers, probably using the lifecycle events (more or less, the same code now it's inside the app.js file)What do you think?
Anyway, congrats for this nice library
The text was updated successfully, but these errors were encountered: