Add test case for string type

[ankurk91]

Version 0.4.0 is no longer working for me.
I rolled back to 0.3.0.

When i run route("valid-route") in console, it returns object rather url (string).
So i am adding a test case for future proofing.

The route() method is expected to return string types if everything goes well.

Something is wrong with existing test cases. Don't know how they get passed.
May be choose a different assertion library.

[DanielCoulbourne]

In what context are you using the route() method? We are testing it with an axios call to ensure that the object is cast to a string.

I can't think of a use case where it wouldn't be cast to a string so I'd love to hear what you're doing to see if there's a way to make this work for you.

[ankurk91]

You can see that my test failed because route() method is not returning string.
Yes, i am using using it with axios and sometimes just to get URL so that i can redirect user .

window.location = route('thank-you');

You can check the gifs.

v0.3.0

v0.4.0

Laravel 5.4.33, Laravel Mix, Chrome 60 stable, Mac OS Seirra

[ankurk91]

I think the culprit is this method,

var Router = function(name, params, absolute) {
    this.name = name;
    this.urlParams = this.normalizeParams(params);
    this.queryParams = this.normalizeParams(params);
    this.absolute = absolute === undefined ? true : absolute;
    this.domain = this.constructDomain();
    this.url = namedRoutes[this.name].uri.replace(/^\//, '');
   // should return something here ?
};

This is not returning anything.

[DanielCoulbourne]

window.location = route('thank-you') should work fine.

The route() method DOES return an object, but when that object is cast to a string it should return the route as it previously did. The reason the other tests pass is because we it is being cast to a string before being compared to the expected strings.

the toString() method is called when route() is called in a context that expects a string. I'm wondering if you are using this in another context and if there is a way we can work around that. For example, I am currently not setting the Router.prototype.valueOf() to anything, so that could be a fix if you have an edge-case.

I don't want to assert that route() returns a string because route() actually DOESN'T return a string. It returns an object which is converted to a string when it needs to be.

[ankurk91]

I think you are talking about toString() magic. How can i forget your tweet 😄 .
https://github.com/tightenco/ziggy/blob/master/src/js/route.js#L83

Now i got your point now, it is working fine.
You may have to update banner gif on readme file.

[DanielCoulbourne]

Good point! I'll do that in the morning 😴

[alepeino]

I can't think of a use case where it wouldn't be cast to a string so I'd love to hear what you're doing to see if there's a way to make this work for you.

I found this to be a problem when passing the route as a prop to a Vue component, which validated for String type.

[DanielCoulbourne]

Any chance you could show me your code so I can see if I can work around it? @alepeino

[ankurk91]

@DanielCoulbourne
I think his code look like this:

<!-- form.blade.php -->
<my-form-component :action="route('form-action')"></my-form-component>

<!-- form.vue -->
<template>
    <form :action="action">
        <!-- form inputs -->
    </form>
</template>
<script>
    export default {
        name: 'my-form-component',
        props: {
            action: {
                type: String,
                required: true
            },
        },
    }
</script>

[alepeino]

👆 that, exactly

[DanielCoulbourne]

Okidoke. I'll try to hack on that use case and see if I can fix it for ya.

In the meantime you should be able to use v0.3.0 with no problem I think.

[alepeino]

@DanielCoulbourne
Sure. At the time I'm keeping 0.4 and doing

<my-form-component :action="route('form-action')->toString()"></my-form-component>

although of course it would be nicer to avoid the need to convert explicitly.

[DanielCoulbourne]

yup! I'll figure this out. Why are you type-checking for String, if you don't mind me asking? Seems like there are a lot of cases where you could accidentally trip that wire.

[alepeino]

Sure, I don't really need the type-check. But what if I don't own the component?

[DanielCoulbourne]

No I totally get that. It just seems like a kind of strict type-check.

So far anywhere that you explicitly check that typeof route() === String you will have a problem. But as far as I know that's going to be true of any String cast-able object. You would probably fail the same test using lots of jQuery functions or whatever.

Ideally you would check that something behaves like a string, since that's what really matters.

If you WERE going to typecheck strings directly (instead of checking that it has the functionality you want) instanceof is better because it will also tell you whether you are dealing with a type that extends the type you are looking for.

if I am checking that something is type Animal and someone passes in new Dog, typeof will fail but instanceof will pass

[alepeino]

Cool, I didn't know about that difference in behavior.
Vue seems to do the validation based on typeof though.

So, it looks like in this case (depending on a 3rd party Vue component that you can't change, which does this kind of strict type checking) explicit route('...')->toString() seems to be the way to go.

[DanielCoulbourne]

Yep. I actually just opened an issue over there to see if we can get that changed: vuejs/vue#6447

[DanielCoulbourne]

Hey everyone! Evan merged the change to Vue so this should be good to go now!

[ankurk91]

Awesome

[simensen]

In my current project, I'm using ziggy to create routes to be posted via axios. It gave me quite a time trying to figure out why my XSRF token was not being set.

Since config.rul ends up as a Router instance, it gets passed to isURLSameOrigin and it fails.

• https://github.com/mzabriskie/axios/blob/master/lib/adapters/xhr.js#L109-L112
• https://github.com/mzabriskie/axios/blob/master/lib/helpers/isURLSameOrigin.js

This was my calling code:

var store_uri = this.owner.type === 'user'
  ? route('api.user.projects.store')
  : route('api.team.projects.store', {team_slug: this.owner.slug});

var form = this.form;
var owner = this.owner;

axios.post(store_uri, form)...

I added an explicit .toString() call and now it works:

axios.post(store_uri.toString(), form)...

Without this, the XSRF TOKEN was not being set correctly and I was getting "Session expired" when trying to use Spark's auth:api guard.

It would be great to better understand how Router instances are coerced into being string types and why projects like axios are not honoring this.

The flexibility of being able to get a Router instance from route is nice, but it appears to be breaking things in odd ways in places where the URL is expected to be a string.

[DanielCoulbourne]

Yeah so I've been Ruminating on this quite a bit. I may create a url() method as an alias of toString() and add that to the official docs.

Alternatively, I may do something like allowing users to set a config variable to determine whether they want to use primitive wrappers or not.

[Harrisonbro]

Yep, +1 on @simensen's point about XSRF — I'm experiencing this too. Obviously, it's frustrating to now have to go and stick toString() on the end of all our uses of route(...)!

Making the return type configurable seems sensible, @DanielCoulbourne. I guess we'd want some way to override the config on a per-case basis? E.g. if ziggy was configured to always return a raw URL string from route('my.route') then one might want to occasionally do something like route('my.route', null, {return: 'wrapper'}).

[jesseleite]

@Harrisonbro just a note that route() already has a 3rd parameter for absolute, to emulate Laravel's absolute param functionality.