From 1b2679ad6144f97a4a09235f64d2042c4d1a4431 Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 9 Oct 2020 10:08:04 -0400 Subject: [PATCH 1/4] Remove with and withQuery, update route(...) to return a literal string when passed a route name --- src/js/route.js | 31 ++------ tests/js/route.test.js | 174 ++++++++++++++++++++--------------------- 2 files changed, 91 insertions(+), 114 deletions(-) diff --git a/src/js/route.js b/src/js/route.js index 39ac20c1..168b111f 100644 --- a/src/js/route.js +++ b/src/js/route.js @@ -126,9 +126,10 @@ class Router extends String { // Get parameters that don't correspond to any route segments to append them to the query const unhandled = Object.keys(this._params) .filter((key) => !this._route.parameterSegments.some(({ name }) => name === key)) + .filter((key) => key !== '_query') .reduce((result, current) => ({ ...result, [current]: this._params[current] }), {}); - return this._route.compile(this._params) + stringify({ ...unhandled, ...this._queryParams }, { + return this._route.compile(this._params) + stringify({ ...unhandled, ...this._params['_query'] }, { addQueryPrefix: true, arrayFormat: 'indices', encodeValuesOnly: true, @@ -201,17 +202,6 @@ class Router extends String { return Object.keys(this._config.namedRoutes).includes(name); } - /** - * Add query parameters to be appended to the compiled URL. - * - * @param {Object} params - * @return {this} - */ - withQuery(params) { - this._queryParams = params; - return this; - } - /** * Parse Laravel-style route parameters of any type into a normalized object. * @@ -282,8 +272,9 @@ class Router extends String { */ _substituteBindings(params, bindings = {}) { return Object.entries(params).reduce((result, [key, value]) => { - // If the value isn't an object there's nothing to substitute, so we return it as-is - if (!value || typeof value !== 'object' || Array.isArray(value)) { + // If the value isn't an object, or if it's an object of explicity query + // parameters, there's nothing to substitute so we return it as-is + if (!value || typeof value !== 'object' || Array.isArray(value) || key === '_query') { return { ...result, [key]: value }; } @@ -346,14 +337,6 @@ class Router extends String { return this.url(); } - /** - * @deprecated since v1.0, pass parameters as the second argument to `route()` instead. - */ - with(params) { - this._params = this._parse(params); - return this; - } - /** * @deprecated since v1.0, use `has()` instead. */ @@ -363,5 +346,7 @@ class Router extends String { } export default function route(name, params, absolute, config) { - return new Router(name, params, absolute, config); + const router = new Router(name, params, absolute, config); + + return name ? router.url() : router; } diff --git a/tests/js/route.test.js b/tests/js/route.test.js index f2174b93..de16cf7f 100644 --- a/tests/js/route.test.js +++ b/tests/js/route.test.js @@ -1,4 +1,4 @@ -import assert, { deepEqual, equal, strictEqual } from 'assert'; +import assert, { deepEqual, strictEqual as same, throws } from 'assert'; import route from '../../src/js/route.js'; const defaultWindow = { @@ -127,43 +127,21 @@ beforeEach(() => { global.Ziggy = { ...defaultZiggy }; }); -describe('string', () => { - test('Router class is a string', () => { - strictEqual(route('posts.index') + '', 'https://ziggy.dev/posts'); - strictEqual(String(route('posts.index')), 'https://ziggy.dev/posts'); - strictEqual(route('posts.index').toString(), 'https://ziggy.dev/posts'); - }); -}); - describe('route()', () => { test('can generate a URL with no parameters', () => { - equal(route('posts.index'), 'https://ziggy.dev/posts'); + same(route('posts.index'), 'https://ziggy.dev/posts'); }); test('can generate a URL with default parameters', () => { - equal(route('translatePosts.index'), 'https://ziggy.dev/en/posts'); - }); - - test('can pass parameters with .with()', () => { - deepEqual(route('posts.show', [1]), route('posts.show').with([1])); - equal(route('posts.show', [1]), route('posts.show').with([1]).url()); - - deepEqual( - route('events.venues.show', { event: 1, venue: 2 }), - route('events.venues.show').with({ event: 1, venue: 2 }) - ); - equal( - route('events.venues.show', { event: 1, venue: 2 }).url(), - route('events.venues.show').with({ event: 1, venue: 2 }).url() - ); + same(route('translatePosts.index'), 'https://ziggy.dev/en/posts'); }); test('can generate a relative URL by passing absolute = false', () => { - equal(route('posts.index', [], false), '/posts'); + same(route('posts.index', [], false), '/posts'); }); test('can generate a URL with filled optional parameters', () => { - equal( + same( route('conversations.show', { type: 'email', subscriber: 123, @@ -174,7 +152,7 @@ describe('route()', () => { }); test('can generate a relative URL with filled optional parameters', () => { - equal( + same( route('conversations.show', { type: 'email', subscriber: 123, @@ -185,21 +163,21 @@ describe('route()', () => { }); test('can generate a relative URL with default parameters', () => { - equal(route('translatePosts.index', [], false), '/en/posts'); + same(route('translatePosts.index', [], false), '/en/posts'); }); test('can error if a required parameter is not provided', () => { - assert.throws(() => route('posts.show').url(), /'post' parameter is required/); + throws(() => route('posts.show').url(), /'post' parameter is required/); }); test('can error if a required parameter is not provided to a route with default parameters', () => { - assert.throws(() => route('translatePosts.show').url(), /'id' parameter is required/); + throws(() => route('translatePosts.show').url(), /'id' parameter is required/); }); test('can error if a required parameter with a default has no default value', () => { global.Ziggy.defaultParameters = {}; - assert.throws( + throws( () => route('translatePosts.index').url(), /'locale' parameter is required/ ); @@ -207,37 +185,37 @@ describe('route()', () => { test('can generate a URL using an integer', () => { // route with required parameters - equal(route('posts.show', 1), 'https://ziggy.dev/posts/1'); + same(route('posts.show', 1), 'https://ziggy.dev/posts/1'); // route with default parameters - equal(route('translatePosts.show', 1), 'https://ziggy.dev/en/posts/1'); + same(route('translatePosts.show', 1), 'https://ziggy.dev/en/posts/1'); }); test('can generate a URL using a string', () => { // route with required parameters - equal(route('posts.show', 'my-first-post'), 'https://ziggy.dev/posts/my-first-post'); + same(route('posts.show', 'my-first-post'), 'https://ziggy.dev/posts/my-first-post'); // route with default parameters - equal(route('translatePosts.show', 'my-first-post'), 'https://ziggy.dev/en/posts/my-first-post'); + same(route('translatePosts.show', 'my-first-post'), 'https://ziggy.dev/en/posts/my-first-post'); }); test('can generate a URL using an object', () => { // routes with required parameters - equal(route('posts.show', { id: 1 }), 'https://ziggy.dev/posts/1'); - equal(route('events.venues.show', { event: 1, venue: 2 }), 'https://ziggy.dev/events/1/venues/2'); + same(route('posts.show', { id: 1 }), 'https://ziggy.dev/posts/1'); + same(route('events.venues.show', { event: 1, venue: 2 }), 'https://ziggy.dev/events/1/venues/2'); // route with optional parameters - equal(route('optionalId', { type: 'model', id: 1 }), 'https://ziggy.dev/optionalId/model/1'); + same(route('optionalId', { type: 'model', id: 1 }), 'https://ziggy.dev/optionalId/model/1'); // route with both required and default parameters - equal(route('translateEvents.venues.show', { event: 1, venue: 2 }), 'https://ziggy.dev/en/events/1/venues/2'); + same(route('translateEvents.venues.show', { event: 1, venue: 2 }), 'https://ziggy.dev/en/events/1/venues/2'); }); test('can generate a URL using an array', () => { // routes with required parameters - equal(route('posts.show', [1]), 'https://ziggy.dev/posts/1'); - equal(route('events.venues.show', [1, 2]), 'https://ziggy.dev/events/1/venues/2'); - equal(route('events.venues.show', [1, 'coliseum']), 'https://ziggy.dev/events/1/venues/coliseum'); + same(route('posts.show', [1]), 'https://ziggy.dev/posts/1'); + same(route('events.venues.show', [1, 2]), 'https://ziggy.dev/events/1/venues/2'); + same(route('events.venues.show', [1, 'coliseum']), 'https://ziggy.dev/events/1/venues/coliseum'); // route with default parameters - equal(route('translatePosts.show', [1]), 'https://ziggy.dev/en/posts/1'); + same(route('translatePosts.show', [1]), 'https://ziggy.dev/en/posts/1'); // route with both required and default parameters - equal(route('translateEvents.venues.show', [1, 2]), 'https://ziggy.dev/en/events/1/venues/2'); + same(route('translateEvents.venues.show', [1, 2]), 'https://ziggy.dev/en/events/1/venues/2'); }); test('can generate a URL using an array of objects', () => { @@ -245,91 +223,80 @@ describe('route()', () => { const venue = { id: 2, name: 'Rogers Centre' }; // route with required parameters - equal(route('events.venues.show', [event, venue]), 'https://ziggy.dev/events/1/venues/2'); + same(route('events.venues.show', [event, venue]), 'https://ziggy.dev/events/1/venues/2'); // route with required and default parameters - equal(route('translateEvents.venues.show', [event, venue]), 'https://ziggy.dev/en/events/1/venues/2'); + same(route('translateEvents.venues.show', [event, venue]), 'https://ziggy.dev/en/events/1/venues/2'); }); test('can generate a URL using an array of integers and objects', () => { const venue = { id: 2, name: 'Rogers Centre' }; // route with required parameters - equal(route('events.venues.show', [1, venue]), 'https://ziggy.dev/events/1/venues/2'); + same(route('events.venues.show', [1, venue]), 'https://ziggy.dev/events/1/venues/2'); // route with required and default parameters - equal(route('translateEvents.venues.show', [1, venue]), 'https://ziggy.dev/en/events/1/venues/2'); + same(route('translateEvents.venues.show', [1, venue]), 'https://ziggy.dev/en/events/1/venues/2'); }); test('can generate a URL for a route with domain parameters', () => { // route with required domain parameters - equal(route('team.user.show', { team: 'tighten', id: 1 }), 'https://tighten.ziggy.dev/users/1'); + same(route('team.user.show', { team: 'tighten', id: 1 }), 'https://tighten.ziggy.dev/users/1'); // route with required domain parameters and default parameters - equal(route('translateTeam.user.show', { team: 'tighten', id: 1 }), 'https://tighten.ziggy.dev/en/users/1'); + same(route('translateTeam.user.show', { team: 'tighten', id: 1 }), 'https://tighten.ziggy.dev/en/users/1'); }); test('can generate a URL for a route with a custom route model binding scope', () => { - equal( + same( route('postComments.show', [ { id: 1, title: 'Post' }, { uuid: 12345, title: 'Comment' }, ]), 'https://ziggy.dev/posts/1/comments/12345' ); - equal( + same( route('postComments.show', [1, { uuid: 'correct-horse-etc-etc' }]), 'https://ziggy.dev/posts/1/comments/correct-horse-etc-etc' ); }); test("can fall back to an 'id' key if an object is passed for a parameter with no registered bindings", () => { - equal(route('translatePosts.update', { id: 14 }), 'https://ziggy.dev/en/posts/14'); - equal(route('translatePosts.update', [{ id: 14 }]), 'https://ziggy.dev/en/posts/14'); - equal(route('events.venues.update', [{ id: 10 }, { id: 1 }]), 'https://ziggy.dev/events/10/venues/1'); + same(route('translatePosts.update', { id: 14 }), 'https://ziggy.dev/en/posts/14'); + same(route('translatePosts.update', [{ id: 14 }]), 'https://ziggy.dev/en/posts/14'); + same(route('events.venues.update', [{ id: 10 }, { id: 1 }]), 'https://ziggy.dev/events/10/venues/1'); }); test('can generate a URL for an app installed in a subfolder', () => { global.Ziggy.baseUrl = 'https://ziggy.dev/subfolder'; - equal( + same( route('postComments.show', [1, { uuid: 'correct-horse-etc-etc' }]), 'https://ziggy.dev/subfolder/posts/1/comments/correct-horse-etc-etc' ); }); test('can error if a route model binding key is missing', () => { - assert.throws( + throws( () => route('postComments.show', [1, { count: 20 }]).url(), /Ziggy error: object passed as 'comment' parameter is missing route model binding key 'uuid'\./ ); }); test('can return base URL if path is "/"', () => { - equal(route('home'), 'https://ziggy.dev'); + same(route('home'), 'https://ziggy.dev'); }); // @todo duplicate test('can ignore an optional parameter', () => { - equal(route('optional', { id: 123 }), 'https://ziggy.dev/optional/123'); - equal(route('optional', { id: 123, slug: 'news' }), 'https://ziggy.dev/optional/123/news'); - equal(route('optional', { id: 123, slug: null }), 'https://ziggy.dev/optional/123'); + same(route('optional', { id: 123 }), 'https://ziggy.dev/optional/123'); + same(route('optional', { id: 123, slug: 'news' }), 'https://ziggy.dev/optional/123/news'); + same(route('optional', { id: 123, slug: null }), 'https://ziggy.dev/optional/123'); }); test('can error if a route name doesn’t exist', () => { - assert.throws(() => route('unknown-route').url(), /Ziggy error: route 'unknown-route' is not in the route list\./); - }); - - test('can append values as a query string with .withQuery', () => { - equal( - route('events.venues.show', [1, 2]).withQuery({ - search: 'rogers', - page: 2, - id: 20, - }), - 'https://ziggy.dev/events/1/venues/2?search=rogers&page=2&id=20' - ); + throws(() => route('unknown-route').url(), /Ziggy error: route 'unknown-route' is not in the route list\./); }); test('can automatically append extra parameter values as a query string', () => { - equal( + same( route('events.venues.show', { event: 1, venue: 2, @@ -338,7 +305,7 @@ describe('route()', () => { }), 'https://ziggy.dev/events/1/venues/2?search=rogers&page=2' ); - equal( + same( route('events.venues.show', { id: 2, event: 1, @@ -348,7 +315,32 @@ describe('route()', () => { 'https://ziggy.dev/events/1/venues/2?id=2&search=rogers' ); // ignore values explicitly set to `null` - equal(route('posts.index', { filled: 'filling', empty: null }), 'https://ziggy.dev/posts?filled=filling'); + same(route('posts.index', { filled: 'filling', empty: null }), 'https://ziggy.dev/posts?filled=filling'); + }); + + test('can explicitly append query parameters using _query parameter', () => { + same( + route('events.venues.show', { + event: 1, + venue: 2, + _query: { + event: 4, + venue: 2, + }, + }), + 'https://ziggy.dev/events/1/venues/2?event=4&venue=2' + ); + same( + route('events.venues.show', { + event: { id: 4, name: 'Fun Event' }, + _query: { + event: 9, + id: 12, + }, + venue: 2, + }), + 'https://ziggy.dev/events/4/venues/2?event=9&id=12' + ); }); test('can generate a URL with a port', () => { @@ -357,25 +349,25 @@ describe('route()', () => { global.Ziggy.basePort = 81; // route with no parameters - equal(route('posts.index'), 'https://ziggy.dev:81/posts'); + same(route('posts.index'), 'https://ziggy.dev:81/posts'); // route with required domain parameters - equal(route('team.user.show', { team: 'tighten', id: 1 }), 'https://tighten.ziggy.dev:81/users/1'); + same(route('team.user.show', { team: 'tighten', id: 1 }), 'https://tighten.ziggy.dev:81/users/1'); }); test('can handle trailing path segments in the base URL', () => { global.Ziggy.baseUrl = 'https://test.thing/ab/cd'; - equal(route('events.venues.index', 1), 'https://test.thing/ab/cd/events/1/venues'); + same(route('events.venues.index', 1), 'https://test.thing/ab/cd/events/1/venues'); }); test('can URL-encode named parameters', () => { global.Ziggy.baseUrl = 'https://test.thing/ab/cd'; - equal( + same( route('events.venues.index', { event: 'Fun&Games' }), 'https://test.thing/ab/cd/events/Fun%26Games/venues' ); - equal( + same( route('events.venues.index', { event: 'Fun&Games', location: 'Blues&Clues', @@ -385,7 +377,7 @@ describe('route()', () => { }); test('can format an array of query parameters', () => { - equal( + same( route('events.venues.index', { event: 'test', guests: ['a', 'b', 'c'], @@ -395,7 +387,7 @@ describe('route()', () => { }); test('can handle a parameter explicity set to `0`', () => { - equal(route('posts.update', 0), 'https://ziggy.dev/posts/0'); + same(route('posts.update', 0), 'https://ziggy.dev/posts/0'); }); test('can accept a custom Ziggy configuration object', () => { @@ -413,7 +405,7 @@ describe('route()', () => { }, }; - equal( + same( route('tightenDev.packages.index', { dev: 1 }, true, config), 'http://notYourAverage.dev/tightenDev/1/packages' ); @@ -492,26 +484,26 @@ describe('current()', () => { test('can get the current route name', () => { global.window.location.pathname = '/events/1/venues/2'; - equal(route().current(), 'events.venues.show'); + same(route().current(), 'events.venues.show'); }); test('can get the current route name on a route with multiple allowed HTTP methods', () => { global.window.location.pathname = '/posts/1'; - equal(route().current(), 'posts.show'); + same(route().current(), 'posts.show'); }); test('can get the current route name with a missing protocol', () => { global.window.location.pathname = '/events/1/venues/'; global.window.location.protocol = ''; - equal(route().current(), 'events.venues.index'); + same(route().current(), 'events.venues.index'); }); test('can ignore query string when getting current route name', () => { global.window.location.pathname = '/events/1/venues?foo=2'; - equal(route().current(), 'events.venues.index'); + same(route().current(), 'events.venues.index'); }); test('can get the current route name with a custom Ziggy object', () => { @@ -531,7 +523,7 @@ describe('current()', () => { }, }; - equal(route(undefined, undefined, undefined, config).current(), 'events.index'); + same(route(undefined, undefined, undefined, config).current(), 'events.index'); }); test('can check the current route name against a pattern', () => { @@ -627,6 +619,6 @@ describe('current()', () => { test('can ignore trailing slashes', () => { global.window.location.pathname = '/events/1/venues/'; - equal(route().current(), 'events.venues.index'); + same(route().current(), 'events.venues.index'); }); }); From 3491aa72d92347f46a5cd69f702e0978d7686af1 Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 9 Oct 2020 10:08:29 -0400 Subject: [PATCH 2/4] Remove unreachable url() method in favour of using toString() directly --- src/js/route.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/js/route.js b/src/js/route.js index 168b111f..8807f09f 100644 --- a/src/js/route.js +++ b/src/js/route.js @@ -118,11 +118,11 @@ class Router extends String { * * @example * // with 'posts.show' route 'posts/{post}' - * route('posts.show', 1).url(); // 'https://ziggy.dev/posts/1' + * (new Router('posts.show', 1)).toString(); // 'https://ziggy.dev/posts/1' * * @return {String} */ - url() { + toString() { // Get parameters that don't correspond to any route segments to append them to the query const unhandled = Object.keys(this._params) .filter((key) => !this._route.parameterSegments.some(({ name }) => name === key)) @@ -329,12 +329,8 @@ class Router extends String { }; } - toString() { - return this.url(); - } - valueOf() { - return this.url(); + return this.toString(); } /** @@ -348,5 +344,5 @@ class Router extends String { export default function route(name, params, absolute, config) { const router = new Router(name, params, absolute, config); - return name ? router.url() : router; + return name ? router.toString() : router; } From 519b99f4eb997fcdf8c1f0092af85d1765b6a89c Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 9 Oct 2020 10:47:29 -0400 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a61d9c2..fd09fae9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Breaking changes are marked with ⚠️. - Use [microbundle](https://github.com/developit/microbundle) instead of Webpack to build and distribute Ziggy ([#312](https://github.com/tighten/ziggy/pull/312)) - ⚠️ Default Ziggy's `baseUrl` to the value of the `APP_URL` environment variable instead of `url('/')` ([#334](https://github.com/tighten/ziggy/pull/334)) - ⚠️ Allow getting the route name with `current()` when the current URL has a query string ([#330](https://github.com/tighten/ziggy/pull/330)) +- ⚠️ Return a literal string from the `route()` function when any arguments are passed to it ([#336](https://github.com/tighten/ziggy/pull/336)) **Deprecated** @@ -39,7 +40,8 @@ Breaking changes are marked with ⚠️. - ⚠️ Remove the following undocumented public properties and methods from the `Router` class returned by the `route()` function ([#330](https://github.com/tighten/ziggy/pull/330)): - `name`, `absolute`, `ziggy`, `urlBuilder`, `template`, `urlParams`, `queryParams`, and `hydrated` - `normalizeParams()`, `hydrateUrl()`, `matchUrl()`, `constructQuery()`, `extractParams()`, `parse()`, and `trimParam()` -- ⚠️ Remove the `UrlBuilder` class ([#330](https://github.com/tighten/ziggy/pull/330)): +- ⚠️ Remove the `UrlBuilder` class ([#330](https://github.com/tighten/ziggy/pull/330)) +- ⚠️ Remove the `url()` method now that `route(...)` returns a string ([#336](https://github.com/tighten/ziggy/pull/336)): **Fixed** From acc2e88eb3daa4b4b7b3164187e2606259bf1199 Mon Sep 17 00:00:00 2001 From: Jacob Baker-Kretzmar Date: Fri, 23 Oct 2020 18:18:09 -0400 Subject: [PATCH 4/4] Remove .url() calls from tests --- tests/js/route.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/js/route.test.js b/tests/js/route.test.js index de16cf7f..d730231b 100644 --- a/tests/js/route.test.js +++ b/tests/js/route.test.js @@ -167,18 +167,18 @@ describe('route()', () => { }); test('can error if a required parameter is not provided', () => { - throws(() => route('posts.show').url(), /'post' parameter is required/); + throws(() => route('posts.show'), /'post' parameter is required/); }); test('can error if a required parameter is not provided to a route with default parameters', () => { - throws(() => route('translatePosts.show').url(), /'id' parameter is required/); + throws(() => route('translatePosts.show'), /'id' parameter is required/); }); test('can error if a required parameter with a default has no default value', () => { global.Ziggy.defaultParameters = {}; throws( - () => route('translatePosts.index').url(), + () => route('translatePosts.index'), /'locale' parameter is required/ ); }); @@ -275,7 +275,7 @@ describe('route()', () => { test('can error if a route model binding key is missing', () => { throws( - () => route('postComments.show', [1, { count: 20 }]).url(), + () => route('postComments.show', [1, { count: 20 }]), /Ziggy error: object passed as 'comment' parameter is missing route model binding key 'uuid'\./ ); }); @@ -292,7 +292,7 @@ describe('route()', () => { }); test('can error if a route name doesn’t exist', () => { - throws(() => route('unknown-route').url(), /Ziggy error: route 'unknown-route' is not in the route list\./); + throws(() => route('unknown-route'), /Ziggy error: route 'unknown-route' is not in the route list\./); }); test('can automatically append extra parameter values as a query string', () => {