pateketrueke/yrv

Proposal, redirect on Router too

Opened this issue · 8 comments

Thanks for your work, yrv is amazing.

Taking your example:

<Router path="/auth">
  {#if !loggedIn}
    <Route exact redirect="/auth/login" />
  {:else}
    <Route exact>Welcome back.</Route>
  {/if}

  <Route path="/protected" condition={() => loggedIn} redirect="/auth/login">O.K.</Route>
  <Route path="/login">Log-in</Route>
</Router>

Why can't we rewrite it like this instead?

<Router path="/auth" condition={() => loggedIn} redirect="/auth/login">
  <Route exact>Welcome back.</Route>
  <Route path="/protected">O.K.</Route>
</Router>
<Router path="/auth" condition={() => !loggedIn} redirect="/auth">
  <Route exact path="/login">Log-in</Route>
</Router>

What do you think about this?

I think we need redirect on Router because I can have many routes like this:

<Router path="/auth" condition={() => loggedIn} redirect="/auth/login">
  <Route exact>Welcome back.</Route>
  <Route path="/protected">O.K.</Route>
  <Route path="/protected/something">O.K.</Route>
  <Route path="/protected/players">O.K.</Route>
  <Route path="/protected/another">O.K.</Route>
</Router>
<Router path="/auth" condition={() => !loggedIn} redirect="/auth">
  <Route exact path="/login">Log-in</Route>
</Router>

Am I wrong?

UPDATE:

I can also use it like this:

<Router condition={() => loggedIn}>
  <Route exact>Welcome back.</Route>
  <Route path="/protected">O.K.</Route>
  <Route path="/protected/something">O.K.</Route>
  <Route path="/protected/players">O.K.</Route>
  <Route path="/protected/another">O.K.</Route>
</Router>
<Router condition={() => !loggedIn}>
  <Route exact redirect="/login" />
  <Route exact path="/login">Login</Route>
</Router>

Updated.

Seems fair to me, let me see what can I do, thanks for your input!

I wrote some rough code to add this in but I can't get your tests to run on my computer.

@jhechtf please rebase from master to pull my changes — just type make ci to fire tests. 💣

It seems like you run this on some variety of Linux -- I can't get that script to run without error. It complains about not finding the browser "chrome:headless".

Please pull again, I'm using Mac and on CI we're using Docker (under linux), you need the latest Chrome version installed to run headless.

I'm on windows :c

It should be fine, as long you get able to run Testcafé on Windows you'll be fine.

In such case just inspect the used npm commands on the Makefile and execute them as you need, I'm not very used on Windows so... good luck!