From cf3a028d4158b6e08673b527503df4c56550cfe9 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Mon, 4 Nov 2024 13:39:15 +0200 Subject: [PATCH 1/7] Change window and document DOM access to self checks for SW context Fixes #2867 --- packages/ts/frontend/src/Connect.ts | 5 +++-- packages/ts/frontend/src/CookieManager.ts | 3 ++- packages/ts/frontend/src/FluxConnection.ts | 3 ++- packages/ts/react-i18n/src/index.ts | 2 +- packages/ts/react-signals/src/FullStackSignal.ts | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/ts/frontend/src/Connect.ts b/packages/ts/frontend/src/Connect.ts index 4b1ae3b282..7369ec04b8 100644 --- a/packages/ts/frontend/src/Connect.ts +++ b/packages/ts/frontend/src/Connect.ts @@ -16,7 +16,7 @@ import { } from './FluxConnection.js'; import type { VaadinWindow } from './types.js'; -const $wnd = window as VaadinWindow; +const $wnd = self as VaadinWindow; $wnd.Vaadin ??= {}; $wnd.Vaadin.registrations ??= []; @@ -300,7 +300,8 @@ export class ConnectClient { throw new TypeError(`2 arguments required, but got only ${arguments.length}`); } - const csrfHeaders = getCsrfTokenHeadersForEndpointRequest(document); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const csrfHeaders = self.document ? getCsrfTokenHeadersForEndpointRequest(self.document) : {}; const headers: Record = { Accept: 'application/json', 'Content-Type': 'application/json', diff --git a/packages/ts/frontend/src/CookieManager.ts b/packages/ts/frontend/src/CookieManager.ts index de4e11cb53..90999e6b69 100644 --- a/packages/ts/frontend/src/CookieManager.ts +++ b/packages/ts/frontend/src/CookieManager.ts @@ -5,7 +5,8 @@ export function calculatePath({ pathname }: URL): string { } const CookieManager: Cookies.CookiesStatic = Cookies.withAttributes({ - path: calculatePath(new URL(document.baseURI)), + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + path: calculatePath(new URL(self.document?.baseURI ?? '/')), }); export default CookieManager; diff --git a/packages/ts/frontend/src/FluxConnection.ts b/packages/ts/frontend/src/FluxConnection.ts index 4fef3a3019..562f5594a0 100644 --- a/packages/ts/frontend/src/FluxConnection.ts +++ b/packages/ts/frontend/src/FluxConnection.ts @@ -182,7 +182,8 @@ export class FluxConnection extends EventTarget { } #connectWebsocket(prefix: string, atmosphereOptions: Partial) { - const extraHeaders = getCsrfTokenHeadersForEndpointRequest(document); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const extraHeaders = self.document ? getCsrfTokenHeadersForEndpointRequest(self.document) : {}; const pushUrl = 'HILLA/push'; const url = prefix.length === 0 ? pushUrl : (prefix.endsWith('/') ? prefix : `${prefix}/`) + pushUrl; this.#socket = atmosphere.subscribe?.({ diff --git a/packages/ts/react-i18n/src/index.ts b/packages/ts/react-i18n/src/index.ts index 5ef8f73227..a46f7e12e0 100644 --- a/packages/ts/react-i18n/src/index.ts +++ b/packages/ts/react-i18n/src/index.ts @@ -31,7 +31,7 @@ export class I18n { constructor() { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (!(window as any).Vaadin?.featureFlags?.hillaI18n) { + if (!(self as any).Vaadin?.featureFlags?.hillaI18n) { // Remove when removing feature flag throw new Error( `The Hilla I18n API is currently considered experimental and may change in the future. To use it you need to explicitly enable it in Copilot or by adding com.vaadin.experimental.hillaI18n=true to vaadin-featureflags.properties`, diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts index 1b96087c8d..58926c971d 100644 --- a/packages/ts/react-signals/src/FullStackSignal.ts +++ b/packages/ts/react-signals/src/FullStackSignal.ts @@ -28,7 +28,7 @@ export abstract class DependencyTrackingSignal extends Signal { protected constructor(value: T | undefined, onFirstSubscribe: () => void, onLastUnsubscribe: () => void) { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (!(window as any).Vaadin?.featureFlags?.fullstackSignals) { + if (!(self as any).Vaadin?.featureFlags?.fullstackSignals) { // Remove when removing feature flag throw new Error( `The Hilla Fullstack Signals API is currently considered experimental and may change in the future. To use it you need to explicitly enable it in Copilot or by adding com.vaadin.experimental.fullstackSignals=true to vaadin-featureflags.properties`, From dfca9e8cc49f1c83f1e26d5113b5d9a47f6feffd Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Mon, 11 Nov 2024 18:47:25 +0200 Subject: [PATCH 2/7] Allow dynamically import atmosphere.js for SW context Fixes #2867 --- packages/ts/frontend/src/FluxConnection.ts | 26 ++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/ts/frontend/src/FluxConnection.ts b/packages/ts/frontend/src/FluxConnection.ts index 562f5594a0..7c38011f52 100644 --- a/packages/ts/frontend/src/FluxConnection.ts +++ b/packages/ts/frontend/src/FluxConnection.ts @@ -1,5 +1,7 @@ import type { ReactiveControllerHost } from '@lit/reactive-element'; -import atmosphere from 'atmosphere.js'; + +import type Atmosphere from 'atmosphere.js'; + import type { Subscription } from './Connect.js'; import { getCsrfTokenHeadersForEndpointRequest } from './CsrfUtils.js'; import { @@ -9,6 +11,8 @@ import { type ServerMessage, } from './FluxMessages.js'; +let atmosphere: Atmosphere.Atmosphere; + export enum State { ACTIVE = 'active', INACTIVE = 'inactive', @@ -71,6 +75,14 @@ type EndpointInfo = { reconnect?(): ActionOnLostSubscription | void; }; +interface ImportMetaEnv { + readonly SW_CONTEXT: boolean; +} + +interface ImportMeta { + readonly env: ImportMetaEnv; +} + /** * A representation of the underlying persistent network connection used for subscribing to Flux type endpoint methods. */ @@ -89,7 +101,17 @@ export class FluxConnection extends EventTarget { constructor(connectPrefix: string, atmosphereOptions?: Partial) { super(); - this.#connectWebsocket(connectPrefix.replace(/connect$/u, ''), atmosphereOptions ?? {}); + // @ts-expect-error - vite environment variable + const meta: ImportMeta = import.meta; + if (!meta.env.SW_CONTEXT) { + (async () => { + atmosphere = await import('atmosphere.js'); + this.#connectWebsocket(connectPrefix.replace(/connect$/u, ''), atmosphereOptions ?? {}); + })().catch((e) => { + // eslint-disable-next-line no-console + console.error('Failed to load atmosphere.js', e); + }); + } } #resubscribeIfWasClosed() { From 784cefe2694d0112d5e4ba3c32d0328422544bf7 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Wed, 13 Nov 2024 09:55:06 +0200 Subject: [PATCH 3/7] Fix dynamic import of atmosphere Fixes #2867 --- packages/ts/frontend/src/FluxConnection.ts | 30 ++++++++-------------- packages/ts/frontend/src/vite-env.d.ts | 11 ++++++++ 2 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 packages/ts/frontend/src/vite-env.d.ts diff --git a/packages/ts/frontend/src/FluxConnection.ts b/packages/ts/frontend/src/FluxConnection.ts index 7c38011f52..fa214fad3d 100644 --- a/packages/ts/frontend/src/FluxConnection.ts +++ b/packages/ts/frontend/src/FluxConnection.ts @@ -1,7 +1,5 @@ import type { ReactiveControllerHost } from '@lit/reactive-element'; - import type Atmosphere from 'atmosphere.js'; - import type { Subscription } from './Connect.js'; import { getCsrfTokenHeadersForEndpointRequest } from './CsrfUtils.js'; import { @@ -75,14 +73,6 @@ type EndpointInfo = { reconnect?(): ActionOnLostSubscription | void; }; -interface ImportMetaEnv { - readonly SW_CONTEXT: boolean; -} - -interface ImportMeta { - readonly env: ImportMetaEnv; -} - /** * A representation of the underlying persistent network connection used for subscribing to Flux type endpoint methods. */ @@ -101,16 +91,16 @@ export class FluxConnection extends EventTarget { constructor(connectPrefix: string, atmosphereOptions?: Partial) { super(); - // @ts-expect-error - vite environment variable - const meta: ImportMeta = import.meta; - if (!meta.env.SW_CONTEXT) { - (async () => { - atmosphere = await import('atmosphere.js'); - this.#connectWebsocket(connectPrefix.replace(/connect$/u, ''), atmosphereOptions ?? {}); - })().catch((e) => { - // eslint-disable-next-line no-console - console.error('Failed to load atmosphere.js', e); - }); + if (!import.meta.env['VITE_SW_CONTEXT']) { + import('atmosphere.js') + .then((module) => { + atmosphere = module.default; + this.#connectWebsocket(connectPrefix.replace(/connect$/u, ''), atmosphereOptions ?? {}); + }) + .catch((error) => { + // eslint-disable-next-line no-console + console.error('Failed to load atmosphere', error); + }); } } diff --git a/packages/ts/frontend/src/vite-env.d.ts b/packages/ts/frontend/src/vite-env.d.ts new file mode 100644 index 0000000000..0384aa0af6 --- /dev/null +++ b/packages/ts/frontend/src/vite-env.d.ts @@ -0,0 +1,11 @@ +// / + +interface ImportMetaEnv { + readonly VITE_SW_CONTEXT: boolean; +} + +interface ImportMeta { + readonly env: ImportMetaEnv; +} + +export {}; From 62fa60858b2728c25d0f8a88dc77af2594d582f5 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Wed, 13 Nov 2024 10:29:03 +0200 Subject: [PATCH 4/7] Fix dynamic import of atmosphere Fixes #2867 --- packages/ts/frontend/src/vite-env.d.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ts/frontend/src/vite-env.d.ts b/packages/ts/frontend/src/vite-env.d.ts index 0384aa0af6..b703640eb2 100644 --- a/packages/ts/frontend/src/vite-env.d.ts +++ b/packages/ts/frontend/src/vite-env.d.ts @@ -1,5 +1,6 @@ // / +// eslint-disable-next-line import/unambiguous interface ImportMetaEnv { readonly VITE_SW_CONTEXT: boolean; } @@ -7,5 +8,3 @@ interface ImportMetaEnv { interface ImportMeta { readonly env: ImportMetaEnv; } - -export {}; From 33b8cc7ffb156b5f032fef84586f8e62e8bdcd73 Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Wed, 13 Nov 2024 16:02:50 +0200 Subject: [PATCH 5/7] Fix dynamic import of atmosphere Fixes #2867 --- packages/ts/frontend/src/FluxConnection.ts | 25 ++++++++++------------ packages/ts/frontend/src/vite-env.d.ts | 2 +- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/ts/frontend/src/FluxConnection.ts b/packages/ts/frontend/src/FluxConnection.ts index fa214fad3d..b7a266ba9b 100644 --- a/packages/ts/frontend/src/FluxConnection.ts +++ b/packages/ts/frontend/src/FluxConnection.ts @@ -9,8 +9,6 @@ import { type ServerMessage, } from './FluxMessages.js'; -let atmosphere: Atmosphere.Atmosphere; - export enum State { ACTIVE = 'active', INACTIVE = 'inactive', @@ -73,6 +71,15 @@ type EndpointInfo = { reconnect?(): ActionOnLostSubscription | void; }; +const initAtmosphere = async () => { + if (!import.meta.env.VITE_SW_CONTEXT) { + return await import('atmosphere.js').then((module) => module.default); + } + return undefined; +}; + +const atmosphere: Atmosphere.Atmosphere | undefined = await initAtmosphere(); + /** * A representation of the underlying persistent network connection used for subscribing to Flux type endpoint methods. */ @@ -91,17 +98,7 @@ export class FluxConnection extends EventTarget { constructor(connectPrefix: string, atmosphereOptions?: Partial) { super(); - if (!import.meta.env['VITE_SW_CONTEXT']) { - import('atmosphere.js') - .then((module) => { - atmosphere = module.default; - this.#connectWebsocket(connectPrefix.replace(/connect$/u, ''), atmosphereOptions ?? {}); - }) - .catch((error) => { - // eslint-disable-next-line no-console - console.error('Failed to load atmosphere', error); - }); - } + this.#connectWebsocket(connectPrefix.replace(/connect$/u, ''), atmosphereOptions ?? {}); } #resubscribeIfWasClosed() { @@ -198,7 +195,7 @@ export class FluxConnection extends EventTarget { const extraHeaders = self.document ? getCsrfTokenHeadersForEndpointRequest(self.document) : {}; const pushUrl = 'HILLA/push'; const url = prefix.length === 0 ? pushUrl : (prefix.endsWith('/') ? prefix : `${prefix}/`) + pushUrl; - this.#socket = atmosphere.subscribe?.({ + this.#socket = atmosphere?.subscribe?.({ contentType: 'application/json; charset=UTF-8', enableProtocol: true, transport: 'websocket', diff --git a/packages/ts/frontend/src/vite-env.d.ts b/packages/ts/frontend/src/vite-env.d.ts index b703640eb2..075fff046b 100644 --- a/packages/ts/frontend/src/vite-env.d.ts +++ b/packages/ts/frontend/src/vite-env.d.ts @@ -2,7 +2,7 @@ // eslint-disable-next-line import/unambiguous interface ImportMetaEnv { - readonly VITE_SW_CONTEXT: boolean; + readonly VITE_SW_CONTEXT?: boolean; } interface ImportMeta { From 3ba43b08d2dab6df9a78ed1db7811233cac366df Mon Sep 17 00:00:00 2001 From: Krisjanis Seglins Date: Wed, 13 Nov 2024 16:30:44 +0200 Subject: [PATCH 6/7] Fix dynamic import of atmosphere as top level await not allowed Env should be optional for other frameworks Fixes #2867 --- packages/ts/frontend/src/FluxConnection.ts | 15 ++++++++------- packages/ts/frontend/src/vite-env.d.ts | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/ts/frontend/src/FluxConnection.ts b/packages/ts/frontend/src/FluxConnection.ts index b7a266ba9b..19d14bd248 100644 --- a/packages/ts/frontend/src/FluxConnection.ts +++ b/packages/ts/frontend/src/FluxConnection.ts @@ -71,14 +71,15 @@ type EndpointInfo = { reconnect?(): ActionOnLostSubscription | void; }; -const initAtmosphere = async () => { - if (!import.meta.env.VITE_SW_CONTEXT) { - return await import('atmosphere.js').then((module) => module.default); - } - return undefined; -}; +let atmosphere: Atmosphere.Atmosphere | undefined; -const atmosphere: Atmosphere.Atmosphere | undefined = await initAtmosphere(); +(async () => { + if (!import.meta.env?.VITE_SW_CONTEXT) { + atmosphere = await import('atmosphere.js').then((module) => module.default); + } +})().catch((e) => { + console.error('Failed to load atmosphere.js', e); +}); /** * A representation of the underlying persistent network connection used for subscribing to Flux type endpoint methods. diff --git a/packages/ts/frontend/src/vite-env.d.ts b/packages/ts/frontend/src/vite-env.d.ts index 075fff046b..91164dbcae 100644 --- a/packages/ts/frontend/src/vite-env.d.ts +++ b/packages/ts/frontend/src/vite-env.d.ts @@ -6,5 +6,5 @@ interface ImportMetaEnv { } interface ImportMeta { - readonly env: ImportMetaEnv; + readonly env?: ImportMetaEnv; } From fbfec0af70a6192fd510c51dfe887df4ca8d3347 Mon Sep 17 00:00:00 2001 From: Vlad Rindevich Date: Mon, 25 Nov 2024 12:53:24 +0200 Subject: [PATCH 7/7] refactor(frontend): use top-level import of Atmosphere --- packages/ts/frontend/src/FluxConnection.ts | 13 +++++++------ packages/ts/frontend/src/vite-env.d.ts | 10 ---------- 2 files changed, 7 insertions(+), 16 deletions(-) delete mode 100644 packages/ts/frontend/src/vite-env.d.ts diff --git a/packages/ts/frontend/src/FluxConnection.ts b/packages/ts/frontend/src/FluxConnection.ts index 19d14bd248..36e5d2ac4e 100644 --- a/packages/ts/frontend/src/FluxConnection.ts +++ b/packages/ts/frontend/src/FluxConnection.ts @@ -1,5 +1,4 @@ import type { ReactiveControllerHost } from '@lit/reactive-element'; -import type Atmosphere from 'atmosphere.js'; import type { Subscription } from './Connect.js'; import { getCsrfTokenHeadersForEndpointRequest } from './CsrfUtils.js'; import { @@ -73,13 +72,15 @@ type EndpointInfo = { let atmosphere: Atmosphere.Atmosphere | undefined; -(async () => { - if (!import.meta.env?.VITE_SW_CONTEXT) { +// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition +if (globalThis.document) { + // In case we are in the browser environment, we have to load atmosphere.js + try { atmosphere = await import('atmosphere.js').then((module) => module.default); + } catch (e: unknown) { + console.error('Failed to load atmosphere.js', e); } -})().catch((e) => { - console.error('Failed to load atmosphere.js', e); -}); +} /** * A representation of the underlying persistent network connection used for subscribing to Flux type endpoint methods. diff --git a/packages/ts/frontend/src/vite-env.d.ts b/packages/ts/frontend/src/vite-env.d.ts deleted file mode 100644 index 91164dbcae..0000000000 --- a/packages/ts/frontend/src/vite-env.d.ts +++ /dev/null @@ -1,10 +0,0 @@ -// / - -// eslint-disable-next-line import/unambiguous -interface ImportMetaEnv { - readonly VITE_SW_CONTEXT?: boolean; -} - -interface ImportMeta { - readonly env?: ImportMetaEnv; -}