Repository: swup/swup
Pull request #597 (2023-01-20 10:20:59)

Ignore visits from loadPage method
**Description**

- Also allow ignoring visits when directly calling `swup.loadPage({ url })`
- If a visit is ignored in `loadPage`, it will force a browser visit via `window.location` (see question below)
- Make the second `popstate` parameter to `loadPage` optional for TS end users as it's mostly internal
- Add a `skipChecks` option to `loadPage` to ensure the `ignoreVisit` option is only called once, e.g. after already checking in the click and popstate handler because their logic requires the check earlier
- Rewrite unit tests with mock functions
- This would be a fix release as it was just an oversight on our part

**Update**

- Added an internal `performPageLoad` method that does not check for ignorable visits
- The public `loadPage` as well as click and popstate handlers will now use the new internal method

Also, some refactoring while at it:
- Added a new `shouldSkipTransition` method to have the logic for that live in a more obvious place
  - Incidentally, that will be useful for the upcoming fragment plugin to disable animations for fragment visits
- Instead of passing around a `popstate` argument, pass around a more generic `event` object
  - Makes sense since the argument to `triggerEvent` is called `originalEvent` and supposed to be generic

**Question**

If `loadPage` tries to load a URL and it learns that it should be ignored, it will hand over to the browser via `window.location`. That seems to make the most sense to me — ignoring a visit from a click event also lets the browser handle the visit. Skipping a popstate event also just let's it through to the browser. The alternative would be doing nothing, in which case `ignoreVisit` would have differing effects across the API. WDYT?

**Checks**

- [x] The PR is submitted to the `master` branch
- [x] The code was linted before pushing (`npm run lint`)
- [x] All tests are passing (`npm run test`)
- [x] New or updated tests are included
- [ ] ~~The documentation was updated as required~~



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6582661..b15f75c 100755
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,11 +2,7 @@
 
 <!-- ## [Unreleased] -->
 
-## [3.0.1] - 2023-01-20
-
-- Fix: remove origin from ignoreVisit parameter
-
-## [3.0.0] - 2023-01-19
+## [3.0.0] - 2023-01
 
 - Support CSS animations and keyframes
 - Allow ignoring visits via callback function
@@ -64,9 +60,8 @@ for details.
 
 - Fix bug where animateHistoryBrowsing option was ignored for OUT animations
 
-[Unreleased]: https://github.com/swup/swup/compare/3.0.1...HEAD
+[Unreleased]: https://github.com/swup/swup/compare/3.0.0...HEAD
 
-[3.0.1]: https://github.com/swup/swup/releases/tag/3.0.1
 [3.0.0]: https://github.com/swup/swup/releases/tag/3.0.0
 [2.0.19]: https://github.com/swup/swup/releases/tag/2.0.19
 [2.0.18]: https://github.com/swup/swup/releases/tag/2.0.18
diff --git a/cypress/fixtures/ignore-visits.html b/cypress/fixtures/ignore-visits.html
index bf2c557..7834230 100644
--- a/cypress/fixtures/ignore-visits.html
+++ b/cypress/fixtures/ignore-visits.html
@@ -10,6 +10,9 @@
 <body>
     <header class="header">
         <ul>
+            <li>
+                <a href="/page-2.html" data-cy="ignore-none">Normal link</a>
+            </li>
             <li>
                 <a href="/page-2.html" data-no-swup data-cy="ignore-element">Ignored link</a>
             </li>
diff --git a/package-lock.json b/package-lock.json
index d1a3e04..88d66ef 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "swup",
-  "version": "3.0.1",
+  "version": "3.0.0",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "swup",
-      "version": "3.0.1",
+      "version": "3.0.0",
       "hasInstallScript": true,
       "license": "MIT",
       "dependencies": {
diff --git a/package.json b/package.json
index 59ae0ea..e5adf16 100755
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "swup",
   "amdName": "Swup",
-  "version": "3.0.1",
+  "version": "3.0.0",
   "description": "Complete, flexible, extensible, and easy-to-use page transition library for your server-side rendered website.",
   "type": "module",
   "source": "src/Swup.ts",
diff --git a/src/Swup.ts b/src/Swup.ts
index 3c0c849..57cf74e 100644
--- a/src/Swup.ts
+++ b/src/Swup.ts
@@ -19,12 +19,12 @@ import { getAnimationPromises } from './modules/getAnimationPromises';
 import { getPageData } from './modules/getPageData';
 import { fetchPage } from './modules/fetchPage';
 import { leavePage } from './modules/leavePage';
-import { loadPage } from './modules/loadPage';
+import { loadPage, performPageLoad } from './modules/loadPage';
 import { replaceContent } from './modules/replaceContent';
 import { on, off, triggerEvent, Handlers } from './modules/events';
 import { use, unuse, findPlugin, Plugin } from './modules/plugins';
 import { renderPage } from './modules/renderPage';
-import { updateTransition } from './modules/updateTransition';
+import { updateTransition, shouldSkipTransition } from './modules/transitions';
 
 import { queryAll } from './utils';
 
@@ -95,6 +95,7 @@ export default class Swup {
 	boundPopStateHandler: (event: PopStateEvent) => void;
 
 	loadPage = loadPage;
+	performPageLoad = performPageLoad;
 	leavePage = leavePage;
 	renderPage = renderPage;
 	replaceContent = replaceContent;
@@ -104,6 +105,7 @@ export default class Swup {
 	on = on;
 	off = off;
 	updateTransition = updateTransition;
+	shouldSkipTransition = shouldSkipTransition;
 	getAnimationPromises = getAnimationPromises;
 	getPageData = getPageData;
 	fetchPage = fetchPage;
@@ -274,7 +276,7 @@ export default class Swup {
 		const customTransition = linkEl.getAttribute('data-swup-transition') || undefined;
 
 		// Finally, proceed with loading the page
-		this.loadPage({ url, customTransition }, null);
+		this.performPageLoad({ url, customTransition });
 	}
 
 	handleLinkToSamePage(url: string, hash: string, event: MouseEvent) {
@@ -315,7 +317,14 @@ export default class Swup {
 			return;
 		}
 
-		const { url, hash } = Location.fromUrl(event.state?.url ?? location.href);
+		const href = event.state?.url ?? location.href;
+
+		// Exit early if the link should be ignored
+		if (this.shouldIgnoreVisit(href)) {
+			return;
+		}
+
+		const { url, hash } = Location.fromUrl(href);
 
 		if (hash) {
 			this.scrollToElement = hash;
@@ -330,7 +339,7 @@ export default class Swup {
 			cleanupAnimationClasses();
 		}
 
-		this.loadPage({ url }, event);
+		this.performPageLoad({ url, event });
 	}
 
 	/**
diff --git a/src/modules/enterPage.ts b/src/modules/enterPage.ts
index 5d95d46..48a5417 100755
--- a/src/modules/enterPage.ts
+++ b/src/modules/enterPage.ts
@@ -1,12 +1,10 @@
 import { nextTick } from '../utils';
 import Swup from '../Swup';
+import { PageRenderOptions } from './renderPage';
 
-export const enterPage = function (
-	this: Swup,
-	{ popstate, skipTransition }: { popstate?: PopStateEvent; skipTransition?: boolean }
-) {
+export const enterPage = function (this: Swup, { event, skipTransition }: PageRenderOptions = {}) {
 	if (skipTransition) {
-		this.triggerEvent('transitionEnd', popstate);
+		this.triggerEvent('transitionEnd', event);
 		this.cleanupAnimationClasses();
 		return [Promise.resolve()];
 	}
@@ -19,7 +17,7 @@ export const enterPage = function (
 	const animationPromises = this.getAnimationPromises('in');
 	Promise.all(animationPromises).then(() => {
 		this.triggerEvent('animationInDone');
-		this.triggerEvent('transitionEnd', popstate);
+		this.triggerEvent('transitionEnd', event);
 		this.cleanupAnimationClasses();
 	});
 	return animationPromises;
diff --git a/src/modules/events.ts b/src/modules/events.ts
index 4976399..1b65b9b 100644
--- a/src/modules/events.ts
+++ b/src/modules/events.ts
@@ -51,11 +51,7 @@ export function off(this: Swup, event?: EventType, handler?: Handler) {
 	}
 }
 
-export function triggerEvent(
-	this: Swup,
-	eventName: EventType,
-	originalEvent?: PopStateEvent | MouseEvent
-): void {
+export function triggerEvent(this: Swup, eventName: EventType, originalEvent?: Event): void {
 	// call saved handlers with "on" method and pass originalEvent object if available
 	this._handlers[eventName].forEach((handler) => {
 		try {
diff --git a/src/modules/leavePage.ts b/src/modules/leavePage.ts
index 2ab0670..63a3e0b 100755
--- a/src/modules/leavePage.ts
+++ b/src/modules/leavePage.ts
@@ -1,13 +1,9 @@
 import Swup from '../Swup';
-import { TransitionOptions } from './loadPage';
+import { PageRenderOptions } from './renderPage';
+
+export const leavePage = function (this: Swup, { event, skipTransition }: PageRenderOptions = {}) {
+	const isHistoryVisit = event instanceof PopStateEvent;
 
-export const leavePage = function (
-	this: Swup,
-	data: TransitionOptions,
-	{ popstate, skipTransition }: { popstate: PopStateEvent | null; skipTransition?: boolean } = {
-		popstate: null
-	}
-) {
 	if (skipTransition) {
 		this.triggerEvent('animationSkipped');
 		return [Promise.resolve()];
@@ -17,7 +13,7 @@ export const leavePage = function (
 
 	// handle classes
 	document.documentElement.classList.add('is-changing', 'is-leaving', 'is-animating');
-	if (popstate) {
+	if (isHistoryVisit) {
 		document.documentElement.classList.add('is-popstate');
 	}
 
diff --git a/src/modules/loadPage.ts b/src/modules/loadPage.ts
index 7d7910e..a733799 100755
--- a/src/modules/loadPage.ts
+++ b/src/modules/loadPage.ts
@@ -7,15 +7,30 @@ export type TransitionOptions = {
 	customTransition?: string;
 };
 
-export const loadPage = function (
-	this: Swup,
-	data: TransitionOptions,
-	popstate: PopStateEvent | null
-) {
-	const { url, customTransition } = data;
-	const skipTransition = !!(popstate && !this.options.animateHistoryBrowsing);
+export type PageLoadOptions = {
+	url: string;
+	event?: Event;
+	customTransition?: string;
+};
 
-	this.triggerEvent('transitionStart', popstate || undefined);
+export function loadPage(this: Swup, data: TransitionOptions) {
+	const { url } = data;
+
+	// Check if the visit should be ignored
+	if (this.shouldIgnoreVisit(url)) {
+		window.location.href = url;
+	} else {
+		this.performPageLoad(data);
+	}
+}
+
+export function performPageLoad(this: Swup, data: PageLoadOptions) {
+	const { url, event, customTransition } = data ?? {};
+
+	const isHistoryVisit = event instanceof PopStateEvent;
+	const skipTransition = this.shouldSkipTransition({ url, event });
+
+	this.triggerEvent('transitionStart', event);
 
 	// set transition object
 	this.updateTransition(getCurrentUrl(), url, customTransition);
@@ -24,10 +39,10 @@ export const loadPage = function (
 	}
 
 	// start/skip animation
-	const animationPromises = this.leavePage(data, { popstate, skipTransition });
+	const animationPromises = this.leavePage({ event, skipTransition });
 
 	// create history record if this is not a popstate call (with or without anchor)
-	if (!popstate) {
+	if (!isHistoryVisit) {
 		createHistoryRecord(url + (this.scrollToElement || ''));
 	}
 
@@ -39,7 +54,7 @@ export const loadPage = function (
 	// when everything is ready, render the page
 	Promise.all<PageRecord | void>([fetchPromise, ...animationPromises])
 		.then(([pageData]) => {
-			this.renderPage(pageData as PageRecord, { popstate, skipTransition });
+			this.renderPage(pageData as PageRecord, { event, skipTransition });
 		})
 		.catch((errorUrl) => {
 			// Return early if errorUrl is not defined (probably aborted preload request)
@@ -54,4 +69,4 @@ export const loadPage = function (
 			// Go back to the actual page we're still at
 			history.go(-1);
 		});
-};
+}
diff --git a/src/modules/renderPage.ts b/src/modules/renderPage.ts
index ccde0ae..78c901d 100755
--- a/src/modules/renderPage.ts
+++ b/src/modules/renderPage.ts
@@ -2,12 +2,15 @@ import { Location, updateHistoryRecord, getCurrentUrl } from '../helpers';
 import Swup from '../Swup';
 import { PageRecord } from './Cache';
 
+export type PageRenderOptions = {
+	event?: Event;
+	skipTransition?: boolean;
+};
+
 export const renderPage = function (
 	this: Swup,
 	page: PageRecord,
-	{ popstate, skipTransition }: { popstate: PopStateEvent | null; skipTransition?: boolean } = {
-		popstate: null
-	}
+	{ event, skipTransition }: PageRenderOptions = {}
 ) {
 	document.documentElement.classList.remove('is-leaving');
 
@@ -30,11 +33,11 @@ export const renderPage = function (
 		document.documentElement.classList.add('is-rendering');
 	}
 
-	this.triggerEvent('willReplaceContent', popstate || undefined);
+	this.triggerEvent('willReplaceContent', event);
 
 	this.replaceContent(page).then(() => {
-		this.triggerEvent('contentReplaced', popstate || undefined);
-		this.triggerEvent('pageView', popstate || undefined);
+		this.triggerEvent('contentReplaced', event);
+		this.triggerEvent('pageView', event);
 
 		// empty cache if it's disabled (in case preload plugin filled it)
 		if (!this.options.cache) {
@@ -42,7 +45,7 @@ export const renderPage = function (
 		}
 
 		// Perform in transition
-		this.enterPage({ popstate: popstate || undefined, skipTransition });
+		this.enterPage({ event, skipTransition });
 
 		// reset scroll-to element
 		this.scrollToElement = null;
diff --git a/src/modules/transitions.ts b/src/modules/transitions.ts
new file mode 100755
index 0000000..bf22882
--- /dev/null
+++ b/src/modules/transitions.ts
@@ -0,0 +1,10 @@
+import Swup from '../Swup';
+
+export function updateTransition(this: Swup, from: string, to: string, custom?: string): void {
+	this.transition = { from, to, custom };
+}
+
+export function shouldSkipTransition(this: Swup, { event }: { url?: string; event?: Event }) {
+	const isHistoryVisit = event instanceof PopStateEvent;
+	return !!(isHistoryVisit && !this.options.animateHistoryBrowsing);
+}
diff --git a/src/modules/updateTransition.ts b/src/modules/updateTransition.ts
deleted file mode 100755
index c1f446f..0000000
--- a/src/modules/updateTransition.ts
+++ /dev/null
@@ -1,10 +0,0 @@
-import Swup from '../Swup';
-
-export const updateTransition = function (
-	this: Swup,
-	from: string,
-	to: string,
-	custom?: string
-): void {
-	this.transition = { from, to, custom };
-};


Test patch:
diff --git a/cypress/e2e/main.spec.js b/cypress/e2e/main.spec.js
index 97008f5..2d2d5bb 100644
--- a/cypress/e2e/main.spec.js
+++ b/cypress/e2e/main.spec.js
@@ -21,10 +21,6 @@ describe('Instance', function () {
 		cy.wrapSwupInstance();
 	});
 
-	it('should have a version property', function () {
-		expect(this.swup.version).to.not.be.empty;
-	});
-
 	it('should mount and unmount plugins', function () {
 		const plugin = createPlugin();
 		cy.spy(plugin, 'mount');
@@ -286,19 +282,19 @@ describe('Ignoring visits', function () {
 	});
 
 	it('should ignore links via custom ignored path', function () {
-		this.swup.options.ignoreVisit = (url) => url.startsWith('/page-2');
+		this.swup.options.ignoreVisit = (url) => url.endsWith('#hash');
 		cy.shouldHaveReloadedAfterAction(() => {
-			cy.get('[data-cy="ignore-path-start"]').first().click();
+			cy.get('[data-cy="ignore-path-end"]').first().click();
 		});
-		cy.shouldBeAtPage('/page-2.html');
+		cy.shouldBeAtPage('/page-2.html#hash');
 	});
 
-	it('should ignore links via custom ignored path', function () {
-		this.swup.options.ignoreVisit = (url) => url.endsWith('.html#hash');
+	it('should ignore visits via loadPage', function () {
+		this.swup.options.ignoreVisit = (url) => true;
 		cy.shouldHaveReloadedAfterAction(() => {
-			cy.get('[data-cy="ignore-path-end"]').first().click();
+			this.swup.loadPage({ url: '/page-2.html' });
 		});
-		cy.shouldBeAtPage('/page-2.html#hash');
+		cy.shouldBeAtPage('/page-2.html');
 	});
 });
 
diff --git a/src/__test__/index.test.ts b/src/__test__/index.test.ts
index cb59143..2adee00 100644
--- a/src/__test__/index.test.ts
+++ b/src/__test__/index.test.ts
@@ -1,3 +1,4 @@
+import pckg from '../../package.json';
 import Swup, { Options, Plugin } from '../index';
 
 console.log = jest.fn();
@@ -32,21 +33,32 @@ describe('exports', () => {
 		};
 
 		const swup = new Swup(options);
+		expect(swup).toBeInstanceOf(Swup);
+	});
 
+	it('defines a version', () => {
+		const swup = new Swup();
 		expect(swup.version).not.toBeUndefined();
+		expect(swup.version).toEqual(pckg.version);
 	});
 
-	it('passes relative URL to ignoreVisit', () => {
-		let ignorableUrl = 'nothing';
-		const swup = new Swup({
-			ignoreVisit: (url, { el } = {}) => {
-				ignorableUrl = url;
-				return false;
-			}
-		});
+	it('calls and passes relative URL to ignoreVisit', () => {
+		const ignoreVisit = jest.fn(() => true);
+		const swup = new Swup({ ignoreVisit });
 
 		swup.shouldIgnoreVisit(baseUrl + '/path/?query#hash');
 
-		expect(ignorableUrl).toEqual('/path/?query#hash');
+		expect(ignoreVisit.mock.calls).toHaveLength(1);
+		expect(ignoreVisit.mock.lastCall).toBeDefined();
+		expect((ignoreVisit.mock.lastCall as any)[0]).toEqual('/path/?query#hash');
+	});
+
+	it('calls ignoreVisit from loadPage method', () => {
+		const ignoreVisit = jest.fn(() => true);
+		const swup = new Swup({ ignoreVisit });
+
+		swup.loadPage({ url: '/path/' });
+
+		expect(ignoreVisit.mock.calls).toHaveLength(1);
 	});
 });