Repository: swup/swup (ts)

Issue:
Feature: Store parsed html document in visit object
### Describe the problem 🧐

The swup.js library currently parses the entire HTML document internally using the DOMParser API. However, the parsed HTML document is not exposed or accessible to the users of the library. The library only returns the HTML document as a string.

Parsing an HTML document is a relatively slow process. In scenarios where the developer needs to work with the parsed HTML document, they have to parse the same HTML document again. This results in unnecessary duplication of work and performance overhead.

To improve the efficiency and flexibility of the library, I propose adding an option that allows users to control whether the parsed HTML document should be included in the visit object or not. When this option is enabled, the visit object should include a reference to the parsed HTML document.

I believe this feature would be a valuable addition to the swup.js library and would greatly improve its usability and performance in certain scenarios.

Thank you for considering this feature request.


### Describe the propsed solution 😎

const swup = new Swup({
  includeParsedHtmlInVisit: true, // New option
  // Other options...
});

swup.hooks.on('content:replace', (visit) => {
  const parsedHtml = visit.to.html.parsed; // Parsed HTML document
  // ...
});

### Alternatives considered 🤔

- Parse the same document string again
- Don't parse it again - search in string using regex


### How important is this feature to you? 🧭

Nice to have

### Checked all these? 📚

- [X] I have read and searched the [official docs](https://swup.js.org)
- [X] I have checked [discussions](https://github.com/orgs/swup/discussions) and [existing issues](https://github.com/issues?q=is:issue+org:swup) for related problems
- [X] I agree to follow this project's [Code of Conduct](https://github.com/swup/.github/blob/master/CODE_OF_CONDUCT.md)
- [X] I have provided all necessary information to the best of my knowledge

Pull request:
Store new document in visit object
**Description**

- Store the parsed `Document` of the new page in the visit object as `visit.to.document`
- Allow reuse in plugins and consumer hooks to improve performance
- Remove the parsed document when the visit ends, is aborted, or throws an exception
- Read the containers from the visit object directly instead of duplicating them as arguments

**Questions**

- Are we rewriting the html in any plugins where we'd also need to rewrite the document?

**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

**Context**

- Closes #862

Patch:
diff --git a/src/modules/Visit.ts b/src/modules/Visit.ts
index 194c9d3..cc7b608 100644
--- a/src/modules/Visit.ts
+++ b/src/modules/Visit.ts
@@ -17,6 +17,8 @@ export interface VisitTo {
 	hash?: string;
 	/** The HTML content of the next page */
 	html?: string;
+	/** The parsed document of the next page, available during visit */
+	document?: Document;
 }
 
 export interface VisitAnimation {
diff --git a/src/modules/navigate.ts b/src/modules/navigate.ts
index 96ee57c..b673379 100755
--- a/src/modules/navigate.ts
+++ b/src/modules/navigate.ts
@@ -79,6 +79,7 @@ export async function performNavigation(
 		} else {
 			// Currently navigating and content not loaded? Abort running visit
 			await this.hooks.call('visit:abort', this.visit, undefined);
+			delete this.visit.to.document;
 			this.visit.state = VisitState.ABORTED;
 		}
 	}
@@ -138,10 +139,14 @@ export async function performNavigation(
 			return args.page;
 		});
 
-		// When page loaded: mark visit as loaded, save html into visit object
+		/**
+		 * When the page is loaded: mark the visit as loaded and save
+		 * the raw html and a parsed document of the received page in the visit object
+		 */
 		page.then(({ html }) => {
 			visit.advance(VisitState.LOADED);
 			visit.to.html = html;
+			visit.to.document = new DOMParser().parseFromString(html, 'text/html');
 		});
 
 		// Create/update history record if this is not a popstate call or leads to the same URL
@@ -229,5 +234,7 @@ export async function performNavigation(
 
 		// Go back to the actual page we're still at
 		window.history.back();
+	} finally {
+		delete visit.to.document;
 	}
 }
diff --git a/src/modules/renderPage.ts b/src/modules/renderPage.ts
index 6be1451..ad89c00 100755
--- a/src/modules/renderPage.ts
+++ b/src/modules/renderPage.ts
@@ -28,7 +28,7 @@ export const renderPage = async function (this: Swup, visit: Visit, page: PageDa
 		if (visit.animation.animate) {
 			this.classes.add('is-rendering');
 		}
-		const success = this.replaceContent(page, { containers: visit.containers });
+		const success = this.replaceContent(visit);
 		if (!success) {
 			throw new Error('[swup] Container mismatch, aborting');
 		}
diff --git a/src/modules/replaceContent.ts b/src/modules/replaceContent.ts
index 8859b95..62859ac 100755
--- a/src/modules/replaceContent.ts
+++ b/src/modules/replaceContent.ts
@@ -1,22 +1,15 @@
 import type Swup from '../Swup.js';
-import type { Options } from '../Swup.js';
 import { query, queryAll } from '../utils.js';
-import type { PageData } from './fetchPage.js';
+import type { Visit } from './Visit.js';
 
 /**
  * Perform the replacement of content after loading a page.
  *
- * It takes an object with the page data as returned from `fetchPage` and a list
- * of container selectors to replace.
- *
  * @returns Whether all containers were replaced.
  */
-export const replaceContent = function (
-	this: Swup,
-	{ html }: PageData,
-	{ containers }: { containers: Options['containers'] } = this.options
-): boolean {
-	const incomingDocument = new DOMParser().parseFromString(html, 'text/html');
+export const replaceContent = function (this: Swup, visit: Visit): boolean {
+	const incomingDocument = visit.to.document;
+	if (!incomingDocument) return false;
 
 	// Update browser title
 	const title = incomingDocument.querySelector('title')?.innerText || '';
@@ -26,12 +19,12 @@ export const replaceContent = function (
 	const persistedElements = queryAll('[data-swup-persist]:not([data-swup-persist=""])');
 
 	// Update content containers
-	const replaced = containers
+	const replaced = visit.containers
 		.map((selector) => {
 			const currentEl = document.querySelector(selector);
 			const incomingEl = incomingDocument.querySelector(selector);
 			if (currentEl && incomingEl) {
-				currentEl.replaceWith(incomingEl);
+				currentEl.replaceWith(incomingEl.cloneNode(true));
 				return true;
 			}
 			if (!currentEl) {
@@ -54,5 +47,5 @@ export const replaceContent = function (
 	});
 
 	// Return true if all containers were replaced
-	return replaced.length === containers.length;
+	return replaced.length === visit.containers.length;
 };


Test patch:
diff --git a/tests/functional/visit-object.spec.ts b/tests/functional/visit-object.spec.ts
index 56c26ff..5ec5142 100644
--- a/tests/functional/visit-object.spec.ts
+++ b/tests/functional/visit-object.spec.ts
@@ -1,6 +1,6 @@
 import { test, expect } from '@playwright/test';
 
-import { clickOnLink, expectToBeAt } from '../support/commands.js';
+import { clickOnLink, expectToBeAt, sleep } from '../support/commands.js';
 import { expectSwupAnimationDuration, navigateWithSwup } from '../support/swup.js';
 
 test.describe('visit object', () => {
@@ -36,6 +36,39 @@ test.describe('visit object', () => {
 		expect(await page.evaluate(() => window.data)).toMatch(/<h1>Page 2/i);
 	});
 
+	test('has the next page document', async ({ page }) => {
+		await page.evaluate(() => {
+			window._swup.hooks.before('content:replace', (visit) => (window.data = visit.to.document));
+		});
+		await navigateWithSwup(page, '/page-2.html');
+		await expectToBeAt(page, '/page-2.html', 'Page 2');
+		await page.waitForSelector('html:not(.is-changing)');
+		expect(await page.evaluate(() => window.data && window.data instanceof Document)).toBe(true);
+	});
+
+	test('removes the next page document when finished', async ({ page }) => {
+		await page.evaluate(() => {
+			window._swup.hooks.on('visit:start', (visit) => (window.data = visit));
+		});
+		await navigateWithSwup(page, '/page-2.html');
+		await expectToBeAt(page, '/page-2.html', 'Page 2');
+		await page.waitForSelector('html:not(.is-changing)');
+		await sleep(20);
+		expect(await page.evaluate(() => window.data.to.document === undefined)).toBe(true);
+	});
+
+	test('removes the next page document when aborted', async ({ page }) => {
+		await page.evaluate(() => {
+			window._swup.hooks.once('visit:start', (visit) => (window.data = visit));
+			window._swup.hooks.once('visit:start', () => window._swup.navigate('/page-3.html'));
+		});
+		await navigateWithSwup(page, '/page-3.html');
+		await expectToBeAt(page, '/page-3.html', 'Page 3');
+		await page.waitForSelector('html:not(.is-changing)');
+		await sleep(20);
+		expect(await page.evaluate(() => window.data.to.document === undefined)).toBe(true);
+	});
+
 	test('passes along click trigger and event', async ({ page }) => {
 		await page.evaluate(() => {
 			window.data = { el: null, event: null };
diff --git a/tests/unit/replaceContent.test.ts b/tests/unit/replaceContent.test.ts
index 726deed..c60e76e 100644
--- a/tests/unit/replaceContent.test.ts
+++ b/tests/unit/replaceContent.test.ts
@@ -1,6 +1,7 @@
-import { describe, expect, it, vi, afterEach } from 'vitest';
+import { describe, expect, it, vi, afterEach, beforeEach } from 'vitest';
 import Swup from '../../src/Swup.js';
 import type { PageData } from '../../src/modules/fetchPage.js';
+import { Visit, createVisit } from '../../src/modules/Visit.js';
 import { JSDOM } from 'jsdom';
 
 const getHtml = (body: string): string => {
@@ -12,19 +13,22 @@ const getHtml = (body: string): string => {
 	`;
 };
 
-const mockPage = (body: string): PageData => {
-	return {
-		url: '',
-		html: getHtml(body)
-	};
-};
-
 const stubGlobalDocument = (body: string): void => {
 	const dom = new JSDOM(getHtml(body));
 	vi.stubGlobal('document', dom.window.document);
 };
 
+class SwupWithPublicVisitMethods extends Swup {
+	public createVisit = createVisit;
+}
+
+const swup = new SwupWithPublicVisitMethods();
+let visit: Visit;
+
 describe('replaceContent', () => {
+	beforeEach(() => {
+		visit = swup.createVisit({ to: '' });
+	});
 	afterEach(() => {
 		vi.unstubAllGlobals();
 	});
@@ -36,12 +40,13 @@ describe('replaceContent', () => {
 			<div id="container-3" data-from="current"></div>
 		`);
 
-		const page = mockPage(/*html*/ `
+		const html = /*html*/ `
 			<div id="container-1" data-from="incoming"></div>
-			<div id="container-2" data-from="incoming"></div>`);
-		const swup = new Swup();
+			<div id="container-2" data-from="incoming"></div>`;
+		visit.to.document = new DOMParser().parseFromString(html, 'text/html')
+		visit.containers = ['#container-1', '#container-2'];
 
-		const result = swup.replaceContent(page, { containers: ['#container-1', '#container-2'] });
+		const result = new Swup().replaceContent(visit);
 
 		expect(result).toBe(true);
 		expect(document.querySelector('#container-1')?.getAttribute('data-from')).toBe('incoming');
@@ -54,13 +59,14 @@ describe('replaceContent', () => {
 			<div id="container-1" data-from="current"></div>
 		`);
 		const warn = vi.spyOn(console, 'warn');
-		const page = mockPage(/*html*/ `
+		const html = /*html*/ `
 			<div id="container-1" data-from="incoming"></div>
 			<div id="container-2" data-from="incoming"></div>
-			`);
+			`;
+		visit.to.document = new DOMParser().parseFromString(html, 'text/html')
+		visit.containers = ['#container-1', '#missing'];
 
-		const swup = new Swup();
-		const result = swup.replaceContent(page, { containers: ['#container-1', '#missing'] });
+		const result = new Swup().replaceContent(visit);
 
 		expect(result).toBe(false);
 		expect(warn).not.toBeCalledWith(
@@ -76,11 +82,11 @@ describe('replaceContent', () => {
 			<div id="container-3" data-from="current"></div>
 		`);
 		const warn = vi.spyOn(console, 'warn');
-		const page = mockPage(/*html*/ `
-			<div id="container-1" data-from="incoming"></div>`);
+		const html = /*html*/ `<div id="container-1" data-from="incoming"></div>`;
+		visit.to.document = new DOMParser().parseFromString(html, 'text/html')
+		visit.containers = ['#container-1', '#missing'];
 
-		const swup = new Swup();
-		const result = swup.replaceContent(page, { containers: ['#container-1', '#missing'] });
+		const result = new Swup().replaceContent(visit);
 
 		expect(result).toBe(false);
 		expect(warn).not.toBeCalledWith(
@@ -88,4 +94,18 @@ describe('replaceContent', () => {
 		);
 		expect(warn).toBeCalledWith('[swup] Container missing in incoming document: #missing');
 	});
+
+	it('should not modify new document in visit object', () => {
+		stubGlobalDocument(/*html*/ `<div id="container" data-from="current"></div>`);
+		const html = /*html*/ `<div id="container" data-from="incoming"></div>`;
+		visit.to.document = new DOMParser().parseFromString(html, 'text/html')
+		visit.containers = ['#container'];
+
+		const documentBefore = visit.to.document.documentElement.innerHTML;
+		new Swup().replaceContent(visit);
+		const documentAfter = visit.to.document.documentElement.innerHTML;
+
+		expect(documentBefore).toEqual(documentAfter);
+		expect(documentAfter).toContain(html);
+	});
 });