inertia icon indicating copy to clipboard operation
inertia copied to clipboard

Link component missing behavior for URLs from another origin

Open MohammadZarifiyan opened this issue 1 year ago • 15 comments

Version:

  • @inertiajs/vue3 version: 1.0.14

Describe the problem:

When I click on a link with a different host, a white screen is displayed and error 409 (Conflict) is written in the console. The Link component should redirect to the specified page if it receives a 409 conflict error.

Solution:

use location.href = linkComponentHref whenever it gets 409 error. This must be implemented inside Link component because all developers expect this component behave like this.

Steps to reproduce:

App.vue:

<script setup>
import {Link} from '@inertiajs/vue3';
</script>

<template>
   <p>Hi this is my website at example.com</p>
   <p>click on following link to see a random page from my website or another website which i don't own</p>
   <Link href="https://google.com">Click here to see Google website</Link>
</template>

MohammadZarifiyan avatar Nov 07 '23 11:11 MohammadZarifiyan

Yeah I don't agree with this. The <Link> element is used to make an Inertia visit, it makes very little sense to make an XHR request to an external party like Google, only to then detect it can't be done (or would be blocked by the browser) to then afterwards make another request, you're just wasting resources then. If you need to redirect and aren't sure if the link is going to be external, you can use Inertia::location($url), as explained here: https://inertiajs.com/redirects#external-redirects

RobertBoes avatar Nov 07 '23 11:11 RobertBoes

Yeah I don't agree with this. The <Link> element is used to make an Inertia visit, it makes very little sense to make an XHR request to an external party like Google, only to then detect it can't be done (or would be blocked by the browser) to then afterwards make another request, you're just wasting resources then. If you need to redirect and aren't sure if the link is going to be external, you can use Inertia::location($url), as explained here: https://inertiajs.com/redirects#external-redirects

I didn't mean to send multiple requests. You only need to send an xhr request once to get the link content. If the received response contains the x-inertia header, the received content can be used in the application. But if the received response does not contain the x-inertia header, it is only necessary to use location.href. In the first case, only one request is sent to the server. In the second case, an additional request is sent to the desired link, which is not important considering that the server does not belong to us.

MohammadZarifiyan avatar Nov 07 '23 14:11 MohammadZarifiyan

Yeah I don't agree with this. The <Link> element is used to make an Inertia visit, it makes very little sense to make an XHR request to an external party like Google, only to then detect it can't be done (or would be blocked by the browser) to then afterwards make another request, you're just wasting resources then. If you need to redirect and aren't sure if the link is going to be external, you can use Inertia::location($url), as explained here: https://inertiajs.com/redirects#external-redirects

In addition, we cannot use inertia::location for all the links inside a website because sometimes it is necessary to place the link of another website in our website for SEO link building.

MohammadZarifiyan avatar Nov 07 '23 14:11 MohammadZarifiyan

I mean, if you're using the <Link> component, you're always making an XHR request first, which just wastes resources for no reason. If you need to link to somewhere externally, like your Google link example, just use a regular anchor tag; <a href="https://google.com/">Click here to see Google website</a>. You don't need to use the Inertia link component for everything, the link component is used to make an Inertia request.

RobertBoes avatar Nov 07 '23 14:11 RobertBoes

I mean, if you're using the <Link> component, you're always making an XHR request first, which just wastes resources for no reason. If you need to link to somewhere externally, like your Google link example, just use a regular anchor tag; <a href="https://google.com/">Click here to see Google website</a>. You don't need to use the Inertia link component for everything, the link component is used to make an Inertia request.

Yes, but using the anchor tag can make things very complicated. For example, I have a component called Footer. Inside the footer, I use a component called FooterLinksGroup. I pass the list of links that should be used in it to the Footer component via App.vue. Then the Footer component gives these links to the FooterLinksGroup component. The links that I gave to the Footer component can include internal and external links. So I have to send a request for each link in the FooterLinksGroup to find out if it is possible to use the Link component or not, and if not, use the anchor tag.

MohammadZarifiyan avatar Nov 07 '23 14:11 MohammadZarifiyan

If anything, I'd argue this behaviour in Inertia makes your life easier instead of more complicated, the Vue Router has the same behaviour. Let me try to explain;

You mentioned SEO, rendering external links separately from internal/app-links would allow you to easily add additional attributes, such as rel="noopener noreferrer" and target="_blank". Inertia deals with Inertia requests, it's a router protocol. So every page visit made with Inertia is expected to return an Inertia response. When you make a request to an external URL this is done through XHR, so the browser sends a HEAD request to the resource which determines if a CORS request can be made. This is then likely blocked by the browser (afaik resulting in JS errors, which would also be caught by Sentry..), only to then make another request by using window.location. This would also mean Inertia would have to be very opinionated in that regard; open it in a new tab or not?

Now, rendering these links is IMHO pretty straightforward, you could either add a property like external: true to those links, or use a simple wrapper component, maybe something like this;

<script setup lang="ts">
import { Link } from '@inertiajs/vue3';
import { computed } from 'vue';

const props = defineProps<{
    url: string
    text: string
}>()

const external = computed(() => window.location.origin !== (new URL(props.url)).origin)
</script>

<template>
    <a v-if="external" rel="noopener noreferrer" target="_blank">
        {{ text }}
    </a>
    <Link href="/" v-else>
        {{ text }}
    </Link>
</template>

RobertBoes avatar Nov 07 '23 15:11 RobertBoes

If anything, I'd argue this behaviour in Inertia makes your life easier instead of more complicated, the Vue Router has the same behaviour. Let me try to explain;

You mentioned SEO, rendering external links separately from internal/app-links would allow you to easily add additional attributes, such as rel="noopener noreferrer" and target="_blank". Inertia deals with Inertia requests, it's a router protocol. So every page visit made with Inertia is expected to return an Inertia response. When you make a request to an external URL this is done through XHR, so the browser sends a HEAD request to the resource which determines if a CORS request can be made. This is then likely blocked by the browser (afaik resulting in JS errors, which would also be caught by Sentry..), only to then make another request by using window.location. This would also mean Inertia would have to be very opinionated in that regard; open it in a new tab or not?

Now, rendering these links is IMHO pretty straightforward, you could either add a property like external: true to those links, or use a simple wrapper component, maybe something like this;

<script setup lang="ts">
import { Link } from '@inertiajs/vue3';
import { computed } from 'vue';

const props = defineProps<{
    url: string
    text: string
}>()

const external = computed(() => window.location.origin !== (new URL(props.url)).origin)
</script>

<template>
    <a v-if="external" rel="noopener noreferrer" target="_blank">
        {{ text }}
    </a>
    <Link href="/" v-else>
        {{ text }}
    </Link>
</template>

OK, But sometimes my app may use several different domains and I'm not aware of them. What should I do then?

MohammadZarifiyan avatar Nov 09 '23 06:11 MohammadZarifiyan

OK, But sometimes my app may use several different domains and I'm not aware of them. What should I do then?

If you're using multiple domains within your own app, the easiest way I found would be to use a middleware, check the origin, if that doesn't match then use an Inertia redirect. Something along the lines of this;

<?php

namespace App\Http\Middleware;

use Closure;
use Illuminate\Http\Request;

class RedirectFromOtherOrigins
{
    public function handle(Request $request, Closure $next)
    {
        if (! $request->inertia()) {
            return $next($request);
        }

        $origin = parse_url($request->headers->get('origin'),  PHP_URL_HOST);
        if ($origin === $request->getHost()) {
            return $next($request);
        }

        return Inertia::redirect($request->fullUrl());
    }
}

RobertBoes avatar Nov 09 '23 09:11 RobertBoes

Yeah. I agree with @RobertBoes on this one. It would add unnecessary lag to (what should be) links.

If this is required behaviour, it's likely preferable to implement a userland solution such as a wrapper component where you can more accurately discern between internal and external links.

A universal solution would introduce unnecessary complexity and likely many edge cases.

craigrileyuk avatar Nov 09 '23 16:11 craigrileyuk

OK but you can embed a wrapper inside InertiaJS.

MohammadZarifiyan avatar Nov 10 '23 17:11 MohammadZarifiyan

OK, But sometimes my app may use several different domains and I'm not aware of them. What should I do then?

If you're using multiple domains within your own app, the easiest way I found would be to use a middleware, check the origin, if that doesn't match then use an Inertia redirect. Something along the lines of this;

<?php

namespace App\Http\Middleware;

use Closure;
use Illuminate\Http\Request;

class RedirectFromOtherOrigins
{
    public function handle(Request $request, Closure $next)
    {
        if (! $request->inertia()) {
            return $next($request);
        }

        $origin = parse_url($request->headers->get('origin'),  PHP_URL_HOST);
        if ($origin === $request->getHost()) {
            return $next($request);
        }

        return Inertia::redirect($request->fullUrl());
    }
}

Didn't work. $origin value is NULL.

MohammadZarifiyan avatar Nov 10 '23 17:11 MohammadZarifiyan

If the purpose of Link component is doing Inertia visit, It must be named Visit component not Link component.

MohammadZarifiyan avatar Nov 10 '23 17:11 MohammadZarifiyan

Didn't work. $origin value is NULL.

Then I guess it should be referrer. It was just an example, to get the idea across.

As for naming, then you could argue about that for days as well. Think Inertia also exports it as InertiaLink, which would align even better with Vue guidelines. The Vue Router is also called a router, and doesn't support external links either.

RobertBoes avatar Nov 10 '23 19:11 RobertBoes

Either way, a few solutions have been proposed here, which are quite easy to implement. Considering the pace of development / merges of Inertia I don't think it would be implemented anytime soon anyways.

I just personally think it's a bad idea to implement this in Inertia, as it's incredibly easy to deal with it in user-land and then there's no need for Inertia to account for many different small use-cases.

RobertBoes avatar Nov 10 '23 19:11 RobertBoes

For svelte use:inertia it would be nice to have a prop to prevent the inertia default/action.

When iterating through links:

{#each links as link}
  {#if !link.external}
    <a use:inertia href={link.href} class="text-slate-500 font-bold underline hover:text-slate-700"> ... </a>
  {:else}
    <a href={link.href} class="text-slate-500 font-bold underline hover:text-slate-700"> ... </a>
  {/if}
{/each}

Feels kind of bad, especially when using a lot of tailwindcss classes since those would have to be duplicated. Something along the lines of

{#each links as link}
    <a use:inertia={{disable: link.external}} href={link.href} class="..."> ... </a>
{/each}

Would be pretty nice. Passing a boolean option into shouldIntercept function perhaps?

manning390 avatar Jan 13 '24 04:01 manning390