Skip to content

Commit

Permalink
[sitecore-jss-nextjs]: the redirects has been fixed #SXA-7405 (#1891)
Browse files Browse the repository at this point in the history 
Co-authored-by: Ruslan Matkovskyi <[email protected]>
  • Loading branch information
@sc-ruslanmatkovskyi
sc-ruslanmatkovskyi and Ruslan Matkovskyi authored Aug 15, 2024
1 parent 2c4ab55 commit 410e85a
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 31 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Our versioning strategy is as follows:
* `[templates/nextjs-sxa]` Fix missing value of field property in Title component. ([#1842](https://github.com/Sitecore/jss/pull/1842))
* `[templates/nextjs-sxa]` The background image in the Container component was being generated from the image ID instead of the mediaUrl parameter. This fix changes that behavior. ([#1879](https://github.com/Sitecore/jss/pull/1879))
* `[templates/nextjs-sxa]` The caption of image component has been fixed. ([#1874](https://github.com/Sitecore/jss/pull/1874))
* `[sitecore-jss-nextjs]` A bug has been fixed in the redirect middleware that occurred when a user clicked on a link rendered by the Link component from the Next.js library(next/link). ([#1876](https://github.com/Sitecore/jss/pull/1876))
* `[sitecore-jss-nextjs]` A bug has been fixed in the redirect middleware that occurred when a user clicked on a link rendered by the Link component from the Next.js library(next/link). ([#1876](https://github.com/Sitecore/jss/pull/1876)) ([#1891](https://github.com/Sitecore/jss/pull/1891))
* `[sitecore-jss-nextjs]` Disable nextjs image optimization in edit and preview modes. This prevents rendering issues in XM Cloud Pages Edit and Preview.

### 🎉 New Features & Improvements
Expand Down
83 changes: 79 additions & 4 deletions packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ describe('RedirectsMiddleware', () => {
const req = createRequest({
nextUrl: {
pathname: '/not-found',
origin: 'http://localhost:3000',
locale: 'en',
href: 'http://localhost:3000/not-found',
clone() {
Expand Down Expand Up @@ -421,6 +422,7 @@ describe('RedirectsMiddleware', () => {
url: {
pathname: 'http://localhost:3000/ua/found',
href: 'http://localhost:3000/not-found',
origin: 'http://localhost:3000',
locale: 'en',
clone: cloneUrl,
},
Expand All @@ -440,6 +442,7 @@ describe('RedirectsMiddleware', () => {
nextUrl: {
pathname: '/not-found',
href: 'http://localhost:3000/not-found',
origin: 'http://localhost:3000',
locale: 'en',
clone: cloneUrl,
},
Expand Down Expand Up @@ -470,6 +473,7 @@ describe('RedirectsMiddleware', () => {
url: {
pathname: 'http://localhost:3000/ua/found',
href: 'http://localhost:3000/not-found',
origin: 'http://localhost:3000',
locale: 'en',
clone: cloneUrl,
},
Expand Down Expand Up @@ -581,6 +585,7 @@ describe('RedirectsMiddleware', () => {
search: '?abc=def',
href: 'http://localhost:3000/not-found?abc=def',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -661,6 +666,67 @@ describe('RedirectsMiddleware', () => {
expect(finalRes).to.deep.equal(res);
});

it('should redirect, when target uses query string', async () => {
const setCookies = () => {};
const res = createResponse({
url: 'http://localhost:3000/found?abc=def',
status: 301,
setCookies,
});
const nextRedirectStub = sinon.stub(NextResponse, 'redirect').callsFake((url, init) => {
const status = typeof init === 'number' ? init : init?.status || 307;
return ({
url,
status,
cookies: { set: setCookies },
headers: res.headers,
} as unknown) as NextResponse;
});
const req = createRequest({
nextUrl: {
pathname: '/not-found',
search: '?abc=def',
href: 'http://localhost:3000/not-found?abc=def',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
},
});

const { middleware, fetchRedirects, siteResolver } = createMiddleware({
pattern: 'not-found',
target: 'http://localhost:3000/found?abc=def',
redirectType: REDIRECT_TYPE_301,
isQueryStringPreserved: false,
locale: 'en',
});

const finalRes = await middleware.getHandler()(req);

validateDebugLog('redirects middleware start: %o', {
hostname: 'foo.net',
language: 'en',
pathname: '/not-found',
});

validateEndMessageDebugLog('redirects middleware end in %dms: %o', {
headers: {},
redirected: undefined,
status: 301,
url: 'http://localhost:3000/found?abc=def',
});

expect(siteResolver.getByHost).to.be.calledWith(hostname);
// eslint-disable-next-line no-unused-expressions
expect(fetchRedirects.called).to.be.true;
expect(finalRes).to.deep.equal(res);
expect(finalRes.status).to.equal(res.status);

nextRedirectStub.restore();
});

xit('should redirect uses token in target', async () => {
const setCookies = () => {};
const res = createResponse({
Expand Down Expand Up @@ -742,6 +808,7 @@ describe('RedirectsMiddleware', () => {
pathname: '/not-found',
href: 'http://localhost:3000/not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -802,6 +869,7 @@ describe('RedirectsMiddleware', () => {
search: 'abc=def',
href: 'http://localhost:3000/not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -971,6 +1039,7 @@ describe('RedirectsMiddleware', () => {
href: 'http://localhost:3000/not-found',
pathname: '/not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -1137,6 +1206,7 @@ describe('RedirectsMiddleware', () => {
pathname: '/not-found',
href: 'http://localhost:3000/not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -1199,6 +1269,7 @@ describe('RedirectsMiddleware', () => {
pathname: '/not-found',
href: 'http://localhost:3000/not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -1258,6 +1329,7 @@ describe('RedirectsMiddleware', () => {
pathname: '/not-found/',
href: 'http://localhost:3000/not-found/',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down Expand Up @@ -1317,17 +1389,19 @@ describe('RedirectsMiddleware', () => {

const req = createRequest({
nextUrl: {
pathname: '/not-found?path=not-found',
href: 'http://localhost:3000/not-found?path=not-found',
pathname: '/not-found',
search: '?path=not-found',
href: 'http://localhost:3000/not-found/?path=not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
},
});

const { middleware, fetchRedirects, siteResolver } = createMiddleware({
pattern: 'not-found',
pattern: '/not-found',
target: 'http://localhost:3000/found',
redirectType: REDIRECT_TYPE_301,
isQueryStringPreserved: false,
Expand All @@ -1339,7 +1413,7 @@ describe('RedirectsMiddleware', () => {
validateDebugLog('redirects middleware start: %o', {
hostname: 'foo.net',
language: 'en',
pathname: '/not-found?path=not-found',
pathname: '/not-found',
});

validateEndMessageDebugLog('redirects middleware end in %dms: %o', {
Expand Down Expand Up @@ -1380,6 +1454,7 @@ describe('RedirectsMiddleware', () => {
search: '?path=not-found',
href: 'http://localhost:3000/not-found/?path=not-found',
locale: 'en',
origin: 'http://localhost:3000',
clone() {
return Object.assign({}, req.nextUrl);
},
Expand Down
46 changes: 20 additions & 26 deletions packages/sitecore-jss-nextjs/src/middleware/redirects-middleware.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,16 @@ export class RedirectsMiddleware extends MiddlewareBase {
);
}

const url = req.nextUrl.clone();
const url = this.normalizeUrl(req.nextUrl.clone());

if (REGEXP_ABSOLUTE_URL.test(existsRedirect.target)) {
url.href = existsRedirect.target;
} else {
const source = this.normalizeUrl(url.pathname, url.search);
const source = `${url.pathname.replace(/\/*$/gi, '')}${url.search}`;
const urlFirstPart = existsRedirect.target.split('/')[1];

if (this.locales.includes(urlFirstPart)) {
url.locale = urlFirstPart;
req.nextUrl.locale = urlFirstPart;
existsRedirect.target = existsRedirect.target.replace(`/${urlFirstPart}`, '');
}

Expand All @@ -130,17 +130,15 @@ export class RedirectsMiddleware extends MiddlewareBase {
.replace(/^\/\//, '/')
.split('?');

url.pathname = `${target[0]}`;

if (target[1]) {
url.search = `?${target[1]}`;
const movedSearchParams = existsRedirect.isQueryStringPreserved
? url.search.replace(/^\?/gi, '&')
: '';
url.search = '?' + new URLSearchParams(`${target[1]}${movedSearchParams}`).toString();
}

const newURL = new URL(
`${url.pathname}${existsRedirect.isQueryStringPreserved ? url.search : ''}`,
url.origin
);
url.href = newURL.href;
const prepareNewURL = new URL(`${target[0]}${url.search}`, url.origin);
url.href = prepareNewURL.href;
}

const redirectUrl = decodeURIComponent(url.href);
Expand Down Expand Up @@ -193,10 +191,7 @@ export class RedirectsMiddleware extends MiddlewareBase {
siteName: string
): Promise<RedirectInfo | undefined> {
const redirects = await this.redirectsService.fetchRedirects(siteName);
const normalizedUrl = new URL(
this.normalizeUrl(req.nextUrl.pathname, req.nextUrl.search || ''),
req.nextUrl.href
);
const normalizedUrl = this.normalizeUrl(req.nextUrl.clone());
const tragetURL = normalizedUrl.pathname;
const targetQS = normalizedUrl.search || '';
const language = this.getLanguage(req);
Expand All @@ -214,7 +209,7 @@ export class RedirectsMiddleware extends MiddlewareBase {

return (
(regexParser(redirect.pattern).test(tragetURL) ||
regexParser(redirect.pattern).test(`${tragetURL}${targetQS}`) ||
regexParser(redirect.pattern).test(`${tragetURL.replace(/\/*$/gi, '')}${targetQS}`) ||
regexParser(redirect.pattern).test(`/${req.nextUrl.locale}${tragetURL}`) ||
regexParser(redirect.pattern).test(
`/${req.nextUrl.locale}${tragetURL}${targetQS}`
Expand All @@ -231,19 +226,18 @@ export class RedirectsMiddleware extends MiddlewareBase {
* When a user clicks on a link generated by the Link component from next/link,
* Next.js adds special parameters in the route called path.
* This method removes these special parameters.
* @param {string} pathname
* @param {string} queryString
* @returns {string} modified url
* @param {URL} url
* @returns {string} normalize url
*/
private normalizeUrl(pathname: string, queryString: string) {
if (!queryString) {
return pathname;
private normalizeUrl(url: URL): URL {
if (!url.search) {
return url;
}

/**
* Prepare special parameters for exclusion.
*/
const splittedPathname = pathname
const splittedPathname = url.pathname
.split('/')
.filter((route) => route)
.map((route) => `path=${route}`);
Expand All @@ -254,7 +248,7 @@ export class RedirectsMiddleware extends MiddlewareBase {
* When a user clicks on this link, Next.js should generate a link for the middleware, formatted like this:
* http://host/about/contact/us?path=about&path=contact&path=us
*/
const newQueryString = queryString
const newQueryString = url.search
.replace(/^\?/, '')
.split('&')
.filter((param) => {
Expand All @@ -266,9 +260,9 @@ export class RedirectsMiddleware extends MiddlewareBase {
.join('&');

if (newQueryString) {
return `${pathname}?${newQueryString}`;
return new URL(`${url.pathname}?${newQueryString}`, url.origin);
}

return pathname;
return new URL(`${url.pathname}`, url.origin);
}
}

0 comments on commit 410e85a

Please sign in to comment.