From e828cd0faa59c57409cdae7e95517f20084af391 Mon Sep 17 00:00:00 2001 From: Princesseuh <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 3 Jan 2024 12:18:32 -0500 Subject: [PATCH 1/6] Revert "feat: support setting rootMargin for `client:visible` (#9363)" This reverts commit 769826edbd9ee2144e9272656dfbe6155b24d67d. --- .changeset/stupid-peas-juggle.md | 10 ---------- packages/astro/src/@types/astro.ts | 6 +++--- packages/astro/src/runtime/client/visible.ts | 8 ++------ 3 files changed, 5 insertions(+), 19 deletions(-) delete mode 100644 .changeset/stupid-peas-juggle.md diff --git a/.changeset/stupid-peas-juggle.md b/.changeset/stupid-peas-juggle.md deleted file mode 100644 index 1e01c0996636..000000000000 --- a/.changeset/stupid-peas-juggle.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -'astro': minor ---- - -Extends the `client:visible` directive by adding an optional `rootMargin` property. This allows a component to be hydrated when it is close to the viewport instead of waiting for it to become visible. - -```html - - -``` diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 74cbb4694d9d..2da7cc141af6 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -65,16 +65,16 @@ export type { export type { RemotePattern } from '../assets/utils/remotePattern.js'; export type { SSRManifest } from '../core/app/types.js'; export type { - AstroCookies, - AstroCookieSetOptions, AstroCookieGetOptions, + AstroCookieSetOptions, + AstroCookies, } from '../core/cookies/index.js'; export interface AstroBuiltinProps { 'client:load'?: boolean; 'client:idle'?: boolean; 'client:media'?: string; - 'client:visible'?: string | boolean; + 'client:visible'?: boolean; 'client:only'?: boolean | string; } diff --git a/packages/astro/src/runtime/client/visible.ts b/packages/astro/src/runtime/client/visible.ts index 9e625ca23df9..de36b29098a5 100644 --- a/packages/astro/src/runtime/client/visible.ts +++ b/packages/astro/src/runtime/client/visible.ts @@ -5,16 +5,12 @@ import type { ClientDirective } from '../../@types/astro.js'; * We target the children because `astro-island` is set to `display: contents` * which doesn't work with IntersectionObserver */ -const visibleDirective: ClientDirective = (load, options, el) => { +const visibleDirective: ClientDirective = (load, _options, el) => { const cb = async () => { const hydrate = await load(); await hydrate(); }; - const ioOptions = { - rootMargin: typeof options.value === 'string' ? options.value : undefined, - }; - const io = new IntersectionObserver((entries) => { for (const entry of entries) { if (!entry.isIntersecting) continue; @@ -23,7 +19,7 @@ const visibleDirective: ClientDirective = (load, options, el) => { cb(); break; // break loop on first match } - }, ioOptions); + }); for (const child of el.children) { io.observe(child); From 8fa9c8c91d3f02db381e75cc5a8e91f8b2de65c5 Mon Sep 17 00:00:00 2001 From: Princesseuh <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 3 Jan 2024 13:45:27 -0500 Subject: [PATCH 2/6] feat: update extended `client:visible` to use an object instead of a string --- .changeset/three-owls-drop.md | 10 ++++++++++ packages/astro/src/@types/astro.ts | 4 +++- packages/astro/src/runtime/client/visible.ts | 13 ++++++++++--- 3 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 .changeset/three-owls-drop.md diff --git a/.changeset/three-owls-drop.md b/.changeset/three-owls-drop.md new file mode 100644 index 000000000000..41b81c41bbe4 --- /dev/null +++ b/.changeset/three-owls-drop.md @@ -0,0 +1,10 @@ +--- +"astro": minor +--- + +Extends the `client:visible` directive by adding an optional `rootMargin` property. This allows a component to be hydrated when it is close to the viewport instead of waiting for it to become visible. + +```astro + + +``` diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 2da7cc141af6..08226ff5e005 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -74,10 +74,12 @@ export interface AstroBuiltinProps { 'client:load'?: boolean; 'client:idle'?: boolean; 'client:media'?: string; - 'client:visible'?: boolean; + 'client:visible'?: ClientVisibleOptions | boolean; 'client:only'?: boolean | string; } +export type ClientVisibleOptions = Pick; + export interface TransitionAnimation { name: string; // The name of the keyframe delay?: number | string; diff --git a/packages/astro/src/runtime/client/visible.ts b/packages/astro/src/runtime/client/visible.ts index de36b29098a5..9be4d9b318a7 100644 --- a/packages/astro/src/runtime/client/visible.ts +++ b/packages/astro/src/runtime/client/visible.ts @@ -1,16 +1,23 @@ -import type { ClientDirective } from '../../@types/astro.js'; +import type { ClientDirective, ClientVisibleOptions } from '../../@types/astro.js'; /** * Hydrate this component when one of it's children becomes visible * We target the children because `astro-island` is set to `display: contents` * which doesn't work with IntersectionObserver */ -const visibleDirective: ClientDirective = (load, _options, el) => { +const visibleDirective: ClientDirective = (load, options, el) => { const cb = async () => { const hydrate = await load(); await hydrate(); }; + const rawOptions = + typeof options.value === 'object' ? (options.value as ClientVisibleOptions) : undefined; + + const ioOptions: IntersectionObserverInit = { + rootMargin: rawOptions?.rootMargin, + }; + const io = new IntersectionObserver((entries) => { for (const entry of entries) { if (!entry.isIntersecting) continue; @@ -19,7 +26,7 @@ const visibleDirective: ClientDirective = (load, _options, el) => { cb(); break; // break loop on first match } - }); + }, ioOptions); for (const child of el.children) { io.observe(child); From e2db526bb6225070856b1558bd822abef12b0550 Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:03:39 -0500 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Nate Moore --- .changeset/three-owls-drop.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/three-owls-drop.md b/.changeset/three-owls-drop.md index 41b81c41bbe4..f6a5f90e9b5a 100644 --- a/.changeset/three-owls-drop.md +++ b/.changeset/three-owls-drop.md @@ -2,9 +2,9 @@ "astro": minor --- -Extends the `client:visible` directive by adding an optional `rootMargin` property. This allows a component to be hydrated when it is close to the viewport instead of waiting for it to become visible. +Adds the ability to set a [`rootMargin`](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserver/rootMargin) setting when using the `client:visible` directive. This allows a component to be hydrated when it is _near_ the viewport, rather than hydrated when it has _entered_ the viewport. ```astro - + ``` From f244bbab1a0dff49710f50c7aef62872d0067146 Mon Sep 17 00:00:00 2001 From: Princesseuh <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:36:47 -0500 Subject: [PATCH 4/6] test: add a test --- .../astro/e2e/custom-client-directives.test.js | 11 ++++++++++- .../custom-client-directives/astro.config.mjs | 18 ++++++++++++++++-- .../custom-client-directives/client-options.js | 6 ++++++ .../src/client-directives-types.d.ts | 5 +++-- .../src/pages/index.astro | 3 ++- 5 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 packages/astro/e2e/fixtures/custom-client-directives/client-options.js diff --git a/packages/astro/e2e/custom-client-directives.test.js b/packages/astro/e2e/custom-client-directives.test.js index fec5ef9a1104..443df43075c1 100644 --- a/packages/astro/e2e/custom-client-directives.test.js +++ b/packages/astro/e2e/custom-client-directives.test.js @@ -1,6 +1,6 @@ import { expect } from '@playwright/test'; -import { testFactory, waitForHydrate } from './test-utils.js'; import testAdapter from '../test/test-adapter.js'; +import { testFactory, waitForHydrate } from './test-utils.js'; const test = testFactory({ root: './fixtures/custom-client-directives/', @@ -89,4 +89,13 @@ function testClientDirectivesShared() { // Hydrated, this should be 1 await expect(counterValue).toHaveText('1'); }); + + test('Client directives should be passed options correctly', async ({ astro, page }) => { + await page.goto(astro.resolveUrl('/')); + + const clientOptions = page.locator('#options'); + await expect(clientOptions).toHaveText( + 'Passed options are: {"message":"Hello! I was passed as an option"}' + ); + }); } diff --git a/packages/astro/e2e/fixtures/custom-client-directives/astro.config.mjs b/packages/astro/e2e/fixtures/custom-client-directives/astro.config.mjs index 3790d21b79bc..ae551477124c 100644 --- a/packages/astro/e2e/fixtures/custom-client-directives/astro.config.mjs +++ b/packages/astro/e2e/fixtures/custom-client-directives/astro.config.mjs @@ -1,9 +1,9 @@ -import { defineConfig } from 'astro/config'; import react from "@astrojs/react"; +import { defineConfig } from 'astro/config'; import { fileURLToPath } from 'node:url'; export default defineConfig({ - integrations: [astroClientClickDirective(), astroClientPasswordDirective(), react()], + integrations: [astroClientClickDirective(), astroClientPasswordDirective(), astroHasOptionsDirective(), react()], }); function astroClientClickDirective() { @@ -33,3 +33,17 @@ function astroClientPasswordDirective() { } }; } + +function astroHasOptionsDirective() { + return { + name: 'astro-options', + hooks: { + 'astro:config:setup': (opts) => { + opts.addClientDirective({ + name: 'options', + entrypoint: fileURLToPath(new URL('./client-options.js', import.meta.url)) + }); + } + } + }; +} diff --git a/packages/astro/e2e/fixtures/custom-client-directives/client-options.js b/packages/astro/e2e/fixtures/custom-client-directives/client-options.js new file mode 100644 index 000000000000..5d21f108cf6e --- /dev/null +++ b/packages/astro/e2e/fixtures/custom-client-directives/client-options.js @@ -0,0 +1,6 @@ +// Hydrate on first click on the window +export default async (load, options) => { + const hydrate = await load() + document.write(`
Passed options are: ${JSON.stringify(options.value)}
`) + await hydrate() +} diff --git a/packages/astro/e2e/fixtures/custom-client-directives/src/client-directives-types.d.ts b/packages/astro/e2e/fixtures/custom-client-directives/src/client-directives-types.d.ts index 07399f7bb09c..6fb3c614d4e0 100644 --- a/packages/astro/e2e/fixtures/custom-client-directives/src/client-directives-types.d.ts +++ b/packages/astro/e2e/fixtures/custom-client-directives/src/client-directives-types.d.ts @@ -1,9 +1,10 @@ declare module 'astro' { interface AstroClientDirectives { 'client:click'?: boolean - 'client:password'?: string + 'client:password'?: string + 'client:options'?: { message: string } } } // Make d.ts a module to similate common packaging setups where the entry `index.d.ts` would augment the types -export {} +export { } diff --git a/packages/astro/e2e/fixtures/custom-client-directives/src/pages/index.astro b/packages/astro/e2e/fixtures/custom-client-directives/src/pages/index.astro index 05c28b109e1c..b03042d44624 100644 --- a/packages/astro/e2e/fixtures/custom-client-directives/src/pages/index.astro +++ b/packages/astro/e2e/fixtures/custom-client-directives/src/pages/index.astro @@ -6,5 +6,6 @@ import Counter from '../components/Counter.jsx'; client:click client:password + client:options - \ No newline at end of file + From 7723c01541f3a5609dc33c178406f62004341ecf Mon Sep 17 00:00:00 2001 From: Princesseuh <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:38:30 -0500 Subject: [PATCH 5/6] nit: comment --- .../e2e/fixtures/custom-client-directives/client-options.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/e2e/fixtures/custom-client-directives/client-options.js b/packages/astro/e2e/fixtures/custom-client-directives/client-options.js index 5d21f108cf6e..8a1a4741e0d6 100644 --- a/packages/astro/e2e/fixtures/custom-client-directives/client-options.js +++ b/packages/astro/e2e/fixtures/custom-client-directives/client-options.js @@ -1,4 +1,4 @@ -// Hydrate on first click on the window +// Hydrate directly and write the passed options to the DOM export default async (load, options) => { const hydrate = await load() document.write(`
Passed options are: ${JSON.stringify(options.value)}
`) From 8cbb65db63ff15bf678cd8f6bb4a722756ff88f0 Mon Sep 17 00:00:00 2001 From: Princesseuh <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 3 Jan 2024 18:19:12 -0500 Subject: [PATCH 6/6] test: write the test some other way to try to convince playwright --- packages/astro/e2e/custom-client-directives.test.js | 3 +++ .../custom-client-directives/client-options.js | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/astro/e2e/custom-client-directives.test.js b/packages/astro/e2e/custom-client-directives.test.js index 443df43075c1..118a5d53f541 100644 --- a/packages/astro/e2e/custom-client-directives.test.js +++ b/packages/astro/e2e/custom-client-directives.test.js @@ -93,6 +93,9 @@ function testClientDirectivesShared() { test('Client directives should be passed options correctly', async ({ astro, page }) => { await page.goto(astro.resolveUrl('/')); + const optionsContent = page.locator('#client-has-options pre'); + await waitForHydrate(page, optionsContent); + const clientOptions = page.locator('#options'); await expect(clientOptions).toHaveText( 'Passed options are: {"message":"Hello! I was passed as an option"}' diff --git a/packages/astro/e2e/fixtures/custom-client-directives/client-options.js b/packages/astro/e2e/fixtures/custom-client-directives/client-options.js index 8a1a4741e0d6..70320cf8182c 100644 --- a/packages/astro/e2e/fixtures/custom-client-directives/client-options.js +++ b/packages/astro/e2e/fixtures/custom-client-directives/client-options.js @@ -1,6 +1,10 @@ // Hydrate directly and write the passed options to the DOM export default async (load, options) => { - const hydrate = await load() - document.write(`
Passed options are: ${JSON.stringify(options.value)}
`) - await hydrate() + const hydrate = await load(); + + const div = document.createElement('div'); + div.id = 'options'; + div.textContent = `Passed options are: ${JSON.stringify(options.value)}`; + document.body.appendChild(div); + await hydrate(); }