Merge pull request #124 from WordPress/limit-hydration

Limit the hydration to block when client-side transitions are deactivated
WordPress · Dec 23, 2022 · 168d09a · 168d09a
2 parents 6994f29 + 94f594f
commit 168d09a
Show file tree

Hide file tree

Showing 19 changed files with 542 additions and 103 deletions.
diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml
@@ -0,0 +1,25 @@
+name: Playwright Tests
+on: pull_request
+jobs:
+    test:
+        timeout-minutes: 60
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/checkout@v3
+            - uses: actions/setup-node@v3
+              with:
+                  node-version: 14
+            - name: Install dependencies
+              run: npm ci
+            - name: Install Playwright Browsers
+              run: npx playwright install --with-deps
+            - name: Build
+              run: npm run build
+            - name: Run Playwright tests
+              run: npx playwright test
+            - uses: actions/upload-artifact@v3
+              if: always()
+              with:
+                  name: playwright-report
+                  path: playwright-report/
+                  retention-days: 30
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,7 @@
 node_modules
 build
 vendor
-composer.lock
+composer.lock
+/test-results/
+/playwright-report/
+/playwright/.cache/
diff --git a/e2e/tovdom-full.html b/e2e/tovdom-full.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+	<head>
+		<title>toVdom - full</title>
+		<meta itemprop="wp-client-side-transitions" content="active" />
+	</head>
+	<body>
+		<div wp-ignore>
+			<wp-show when="state.falseValue">
+				<span data-testid="inside wp-ignore">
+					This should not be shown because we are in full mode.
+				</span>
+			</wp-show>
+		</div>
+
+		<script src="../build/e2e/tovdom.js"></script>
+		<script src="../build/runtime.js"></script>
+		<script src="../build/vendors.js"></script>
+	</body>
+</html>
diff --git a/e2e/tovdom-full.spec.ts b/e2e/tovdom-full.spec.ts
@@ -0,0 +1,13 @@
+import { join } from 'path';
+import { test, expect } from '@playwright/test';
+
+test.describe('toVdom - full', () => {
+	test.beforeEach(async ({ page }) => {
+		await page.goto('file://' + join(__dirname, 'tovdom-full.html'));
+	});
+
+	test('it should stop when it founds wp-ignore', async ({ page }) => {
+		const el = page.getByTestId('inside wp-ignore');
+		await expect(el).toBeVisible();
+	});
+});
diff --git a/e2e/tovdom-islands.html b/e2e/tovdom-islands.html
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+	<head>
+		<title>toVdom - islands</title>
+	</head>
+	<body>
+		<div>
+			<wp-show when="state.falseValue">
+				<span data-testid="not inside an island">
+					This should be shown because it is inside an island.
+				</span>
+			</wp-show>
+
+			<div wp-island>
+				<wp-show when="state.falseValue">
+					<span data-testid="inside an island">
+						This should not be shown because it is inside an island.
+					</span>
+				</wp-show>
+			</div>
+
+			<div wp-island>
+				<div wp-ignore>
+					<wp-show when="state.falseValue">
+						<span
+							data-testid="inside an inner block of an isolated island"
+						>
+							This should be shown because it is inside an inner
+							block of an isolated island.
+						</span>
+					</wp-show>
+				</div>
+			</div>
+
+			<div wp-island>
+				<div wp-island>
+					<wp-show
+						when="state.falseValue"
+						data-testid="island inside another island"
+					>
+						<span>
+							This should not have two template wrappers because
+							that means we hydrated twice.
+						</span>
+					</wp-show>
+				</div>
+			</div>
+
+			<div wp-island>
+				<div>
+					<div wp-island wp-ignore>
+						<wp-show when="state.falseValue">
+							<span
+								data-testid="island inside inner block of isolated island"
+							>
+								This should not be shown because even though it
+								is inside an inner block of an isolated island,
+								it's inside an new island.
+							</span>
+						</wp-show>
+					</div>
+				</div>
+			</div>
+		</div>
+
+		<script src="../build/e2e/tovdom.js"></script>
+		<script src="../build/runtime.js"></script>
+		<script src="../build/vendors.js"></script>
+	</body>
+</html>
diff --git a/e2e/tovdom-islands.spec.ts b/e2e/tovdom-islands.spec.ts
@@ -0,0 +1,48 @@
+import { join } from 'path';
+import { test, expect } from '@playwright/test';
+
+test.describe('toVdom - isands', () => {
+	test.beforeEach(async ({ page }) => {
+		await page.goto('file://' + join(__dirname, 'tovdom-islands.html'));
+	});
+
+	test('directives that are not inside islands should not be hydrated', async ({
+		page,
+	}) => {
+		const el = page.getByTestId('not inside an island');
+		await expect(el).toBeVisible();
+	});
+
+	test('directives that are inside islands should be hydrated', async ({
+		page,
+	}) => {
+		const el = page.getByTestId('inside an island');
+		await expect(el).toBeHidden();
+	});
+
+	test('directives that are inside inner blocks of isolated islands should not be hydrated', async ({
+		page,
+	}) => {
+		const el = page.getByTestId(
+			'inside an inner block of an isolated island'
+		);
+		await expect(el).toBeVisible();
+	});
+
+	test('directives inside islands should not be hydrated twice', async ({
+		page,
+	}) => {
+		const el = page.getByTestId('island inside another island');
+		const templates = el.locator('template');
+		expect(await templates.count()).toEqual(1);
+	});
+
+	test('islands inside inner blocks of isolated islands should be hydrated', async ({
+		page,
+	}) => {
+		const el = page.getByTestId(
+			'island inside inner block of isolated island'
+		);
+		await expect(el).toBeHidden();
+	});
+});
diff --git a/e2e/tovdom.js b/e2e/tovdom.js
@@ -0,0 +1,7 @@
+import wpx from '../src/runtime/wpx';
+
+wpx({
+	state: {
+		falseValue: false,
+	},
+});
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -30,6 +30,7 @@
 	"devDependencies": {
 		"@babel/core": "^7.17.10",
 		"@babel/preset-env": "^7.17.10",
+		"@playwright/test": "^1.29.0",
 		"@prettier/plugin-php": "^0.18.9",
 		"@types/jest": "^27.5.1",
 		"@wordpress/env": "^4.4.0",

diff --git a/playwright.config.ts b/playwright.config.ts
@@ -0,0 +1,107 @@
+import type { PlaywrightTestConfig } from '@playwright/test';
+import { devices } from '@playwright/test';
+
+/**
+ * Read environment variables from file.
+ * https://github.com/motdotla/dotenv
+ */
+// require('dotenv').config();
+
+/**
+ * See https://playwright.dev/docs/test-configuration.
+ */
+const config: PlaywrightTestConfig = {
+	testDir: './e2e',
+	/* Maximum time one test can run for. */
+	timeout: 30 * 1000,
+	expect: {
+		/**
+		 * Maximum time expect() should wait for the condition to be met.
+		 * For example in `await expect(locator).toHaveText();`
+		 */
+		timeout: 5000,
+	},
+	/* Run tests in files in parallel */
+	fullyParallel: true,
+	/* Fail the build on CI if you accidentally left test.only in the source code. */
+	forbidOnly: !!process.env.CI,
+	/* Retry on CI only */
+	retries: process.env.CI ? 2 : 0,
+	/* Opt out of parallel tests on CI. */
+	workers: process.env.CI ? 1 : undefined,
+	/* Reporter to use. See https://playwright.dev/docs/test-reporters */
+	reporter: 'html',
+	/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
+	use: {
+		/* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
+		actionTimeout: 0,
+		/* Base URL to use in actions like `await page.goto('/')`. */
+		// baseURL: 'http://localhost:3000',
+
+		/* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
+		trace: 'on-first-retry',
+	},
+
+	/* Configure projects for major browsers */
+	projects: [
+		{
+			name: 'chromium',
+			use: {
+				...devices['Desktop Chrome'],
+			},
+		},
+
+		{
+			name: 'firefox',
+			use: {
+				...devices['Desktop Firefox'],
+			},
+		},
+
+		{
+			name: 'webkit',
+			use: {
+				...devices['Desktop Safari'],
+			},
+		},
+
+		/* Test against mobile viewports. */
+		// {
+		//   name: 'Mobile Chrome',
+		//   use: {
+		//     ...devices['Pixel 5'],
+		//   },
+		// },
+		// {
+		//   name: 'Mobile Safari',
+		//   use: {
+		//     ...devices['iPhone 12'],
+		//   },
+		// },
+
+		/* Test against branded browsers. */
+		// {
+		//   name: 'Microsoft Edge',
+		//   use: {
+		//     channel: 'msedge',
+		//   },
+		// },
+		// {
+		//   name: 'Google Chrome',
+		//   use: {
+		//     channel: 'chrome',
+		//   },
+		// },
+	],
+
+	/* Folder for test artifacts such as screenshots, videos, traces, etc. */
+	// outputDir: 'test-results/',
+
+	/* Run your local dev server before starting the tests */
+	// webServer: {
+	//   command: 'npm run start',
+	//   port: 3000,
+	// },
+};
+
+export default config;
diff --git a/src/admin/admin-page.php b/src/admin/admin-page.php
@@ -11,16 +11,16 @@ function wp_directives_register_menu() {
 }
 add_action( 'admin_menu', 'wp_directives_register_menu' );
 
-function wp_directives_render_admin_page() {?>
+function wp_directives_render_admin_page() {    ?>
 	<div class="wrap">
-		<h2>WP Directives</h2>
-		<form method="POST" action="options.php">
-			<?php
-				settings_fields( 'wp_directives_plugin_settings' );
-				do_settings_sections( 'wp_directives_plugin_page' );
+	  <h2>WP Directives</h2>
+	  <form method="POST" action="options.php">
+		  <?php
+			settings_fields( 'wp_directives_plugin_settings' );
+			do_settings_sections( 'wp_directives_plugin_page' );
 			?>
-			<?php submit_button(); ?>
-		</form>
+		  <?php submit_button(); ?>
+	  </form>
 	</div>
 	<?php
 }
@@ -67,13 +67,7 @@ function wp_directives_client_side_transitions_input() {
 	?>
 
 	<input type="checkbox" 
-		name="
-		<?php
-		echo esc_attr(
-			'wp_directives_plugin_settings[client_side_transitions]'
-		)
-		?>
-		" 
+		name="<?php echo esc_attr( 'wp_directives_plugin_settings[client_side_transitions]' ); ?>" 
 		<?php echo $options['client_side_transitions'] ? 'checked' : ''; ?>
 	>