Skip to content
This repository has been archived by the owner on Apr 29, 2023. It is now read-only.

WIP: BasePath #70

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP: BasePath #70

wants to merge 2 commits into from

Conversation

kidandcat
Copy link

Include Basepath

Jairo Caro-Accino Viciana added 2 commits April 25, 2018 18:29
fix
fix unchecked variables
@codecov
Copy link

codecov bot commented Apr 25, 2018

Codecov Report

Merging #70 into master will decrease coverage by 4.05%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
- Coverage     100%   95.94%   -4.06%     
==========================================
  Files           6        6              
  Lines          68       74       +6     
  Branches       11       14       +3     
==========================================
+ Hits           68       71       +3     
- Misses          0        3       +3
Impacted Files Coverage Δ
src/Route.js 70% <57.14%> (-30%) ⬇️

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 8db6747...65b39fd. Read the comment docs.

@Swizz
Copy link
Contributor

Swizz commented Apr 25, 2018

What is the benefit of the basepath property comparing to this simple concatenation ?

const basepath = "demo"
// ...
<Route path={`${basepath}/topic`} render={Topic} />

@jorgebucaran
Copy link
Owner

@kidandcat

I am sure your idea is well intended Jairo, but do you think is it that useful to most Hyperapp Router users? I am curious, but not very keen on adding this feature, at least not at first sight. 🤔

@kidandcat
Copy link
Author

kidandcat commented Apr 26, 2018

I'm having a separate hyperapp app as a widget: https://github.com/kidandcat/newsuncork/blob/master/src/apps/product-creator.jsx

And I'm instancing it here: https://github.com/kidandcat/newsuncork/blob/master/src/components/CreateProduct.jsx

I'll investigate why today, but when it loads, the location of CreateProduct.jsx's router is "/" (but the URL path is /admin/product/create)

PD: I've added "WIP" to the PR name, if I find the reason of why the router is getting the initial path as "/" I will write you here. However, if at the end this is not usefull to most users, I will just close the PR and use my fork.

@kidandcat kidandcat changed the title BasePath WIP: BasePath Apr 26, 2018
@jorgebucaran
Copy link
Owner

@kidandcat We should add features to the Router which are impossible to add from the outside. Couldn't you solve this problem in userland?

@kidandcat
Copy link
Author

I couldn't do it in the userland, but I managed to do it in a much more cleaner way:

#72

@jorgebucaran

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

Successfully merging this pull request may close these issues.

3 participants