Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes linting across the monorepo (moving to ESLint v9 flat config) and applies a wide set of formatting/type-import fixes, with a few refactors in React hooks/components to satisfy stricter lint rules.
Changes:
- Introduces shared ESLint flat-config (
eslint.base.mjs) and per-packageeslint.config.mjs, plus workspacelintscripts. - Applies repo-wide lint-driven edits (type-only imports, trailing commas, object key ordering, arrow-function conversions, etc.).
- Refactors parts of
uix-host-reacthooks/components to address React hook dependency and lint constraints.
Reviewed changes
Copilot reviewed 70 out of 73 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/uix-host/src/utils/comparePackagesVersions.ts | Formatting changes to satisfy lint rules (expanded if blocks). |
| packages/uix-host/src/utils/compareExtensions.ts | Type-only imports + stable string sorting with localeCompare. |
| packages/uix-host/src/port.ts | Lint/format updates; minor refactors (regex exec, arrow typeguards, parameter commas). |
| packages/uix-host/src/port.test.ts | Lint/format updates and import reordering for mocked connectIframe. |
| packages/uix-host/src/metrics.ts | Lint cleanup and minor refactors in metrics wrapper + runtime polling formatting. |
| packages/uix-host/src/host.ts | Type-only imports and formatting; minor statement/arg ordering adjustments. |
| packages/uix-host/src/extensions-provider/mute.ts | Type-only import + trailing comma to satisfy lint rules. |
| packages/uix-host/src/extensions-provider/extension-registry.ts | Type-only imports + formatting; minor lint suppressions. |
| packages/uix-host/src/extensions-provider/extension-registry.test.ts | Test formatting changes and lint suppression for require. |
| packages/uix-host/src/extensions-provider/composition.ts | Type-only imports + simplified arrow return. |
| packages/uix-host/src/dom-utils/iframe-normalizers.ts | Type-only imports split + formatting adjustments. |
| packages/uix-host/src/dom-utils/iframe-normalizers.test.ts | Formatting fixes (trailing commas, blank lines). |
| packages/uix-host/src/dom-utils/attribute-normalizers.ts | Minor formatting (padding lines). |
| packages/uix-host/src/dom-utils/attribute-normalizers.test.ts | Import reordering + formatting. |
| packages/uix-host/src/debug-host.ts | Type-only imports split + formatting updates. |
| packages/uix-host/package.json | Adds lint script for package-local eslint runs. |
| packages/uix-host/eslint.config.mjs | Adds package ESLint flat-config entrypoint. |
| packages/uix-host-react/src/hooks/useHost.ts | Converts to export const + type-only import cleanup. |
| packages/uix-host-react/src/hooks/useExtensions.ts | Refactors memo/deps handling, subscriptions, and unload handling to satisfy hooks lint. |
| packages/uix-host-react/src/hooks/useExtensions.test.tsx | Import cleanup and formatting updates for lint. |
| packages/uix-host-react/src/hooks/useExtensionListFetched.ts | Converts to export const + type-only import cleanup. |
| packages/uix-host-react/src/extension-context.ts | Type-only import cleanup + formatting. |
| packages/uix-host-react/src/components/GuestUIFrame.tsx | Refactors connection/resizing effects; removes default export; adds memo/callback refs. |
| packages/uix-host-react/src/components/ExtensibleWrapper/UrlExtensionProvider.ts | Type-only imports + functional style conversions; formatting updates. |
| packages/uix-host-react/src/components/ExtensibleWrapper/UrlExtensionProvider.test.ts | Test updates (https URLs, formatting, import changes). |
| packages/uix-host-react/src/components/ExtensibleWrapper/ExtensionManagerProvider.ts | Type-only imports + functional style conversions; formatting updates. |
| packages/uix-host-react/src/components/ExtensibleWrapper/ExtensionManagerProvider.test.ts | Test restructuring/formatting + global fetch mocking adjustments. |
| packages/uix-host-react/src/components/ExtensibleWrapper/ExtensibleWrapper.tsx | Type-only imports + formatting; adjusts scope typing to Record<string,string>. |
| packages/uix-host-react/src/components/ExtensibleComponentBoundary.tsx | Type-only import cleanup + minor component formatting; react-refresh lint suppression. |
| packages/uix-host-react/src/components/Extensible.tsx | Refactors helper + effects/deps to satisfy hooks lint; removes default export. |
| packages/uix-host-react/src/components/Extensible.test.tsx | Formatting + lint suppressions (max-lines-per-function) and minor mock reshaping. |
| packages/uix-host-react/package.json | Adds lint script (with --fix). |
| packages/uix-host-react/eslint.config.mjs | Adds React-enabled package ESLint flat-config entrypoint. |
| packages/uix-guest/src/index.ts | Import ordering/type-only adjustments + formatting. |
| packages/uix-guest/src/guest.ts | Formatting + changes around timeouts and postMessage payload ordering. |
| packages/uix-guest/src/guest-ui.ts | Import splitting (type vs value) + formatting + declare ordering. |
| packages/uix-guest/src/guest-server.ts | Import ordering/type-only tweaks. |
| packages/uix-guest/src/debug-guest.ts | Type-only import split + formatting. |
| packages/uix-guest/package.json | Adds lint script for package-local eslint runs. |
| packages/uix-guest/eslint.config.mjs | Adds package ESLint flat-config entrypoint. |
| packages/uix-core/src/value-assertions.ts | Formatting + trailing commas. |
| packages/uix-core/src/types.ts | Formatting + trailing commas + generic formatting. |
| packages/uix-core/src/tunnel/tunnel.ts | Refactors cleanup placement + adds lint suppressions (random, ignored exceptions). |
| packages/uix-core/src/tunnel/tunnel.test.ts | Formatting + minor data object key ordering. |
| packages/uix-core/src/tunnel/tunnel-messenger.ts | Safer hasOwnProperty usage + type-only imports split + formatting. |
| packages/uix-core/src/tunnel/tunnel-messenger.test.ts | Formatting + trailing commas. |
| packages/uix-core/src/rpc/call-sender.ts | Formatting + adds max-params suppression; minor ordering changes. |
| packages/uix-core/src/rpc/call-sender.test.ts | Import cleanup + formatting; avoids deprecated matcher style warnings. |
| packages/uix-core/src/rpc/call-receiver.ts | Formatting + argument ordering in response tickets. |
| packages/uix-core/src/rpc/call-receiver.test.ts | Import cleanup + formatting. |
| packages/uix-core/src/remote-subject.ts | Import order/type-only adjustments + formatting. |
| packages/uix-core/src/promises/timed.ts | Formatting + trailing commas. |
| packages/uix-core/src/promises/promise-wrappers.test.ts | Formatting + avoids deprecated matcher style warnings. |
| packages/uix-core/src/object-walker.ts | Type cleanup and formatting; refines function type constraints. |
| packages/uix-core/src/object-simulator.ts | Refactors receiver/sender creation; adjusts binding/caching logic; formatting/lint suppressions. |
| packages/uix-core/src/object-simulator.test.ts | Snapshot/test updates due to simulator changes + formatting. |
| packages/uix-core/src/namespace-proxy.ts | Type-only import + formatting + minor key ordering. |
| packages/uix-core/src/message-wrapper.ts | Formatting + trailing commas. |
| packages/uix-core/src/logging-formatters.ts | Adds explicit ignored-exception suppression; formatting. |
| packages/uix-core/src/emitter.ts | Switches to type-only import + formatting + trailing commas. |
| packages/uix-core/src/debuglog.ts | Formatting and some property ordering changes; minor refactors. |
| packages/uix-core/src/debug-emitter.ts | Type-only imports split + formatting and payload ordering change. |
| packages/uix-core/src/cross-realm-object.ts | Formatting + explicit function casting; trailing commas. |
| packages/uix-core/src/mocks/mock-finalization-registry.ts | Adds jest global comment + lint suppression for static mock. |
| packages/uix-core/src/helpers/jest.messagechannel.cjs | Adds eslint-disable header for CJS require + node globals. |
| packages/uix-core/package.json | Adds lint script for package-local eslint runs. |
| packages/uix-core/eslint.config.mjs | Adds package ESLint flat-config entrypoint. |
| package.json | Adds workspace lint runner + upgrades eslint/prettier toolchain deps to support flat config. |
| eslint.base.mjs | New shared ESLint flat configuration factory used by packages. |
| e2e/all-versions/host-app/package-lock.json | Updates resolved registry URLs for e2e lockfile dependencies. |
| e2e/all-versions/guest-app/package-lock.json | Updates resolved registry URLs for e2e lockfile dependencies. |
| .eslintrc.cjs | Removes legacy ESLint config in favor of flat config. |
Files not reviewed (2)
- e2e/all-versions/guest-app/package-lock.json: Language not supported
- e2e/all-versions/host-app/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)
packages/uix-guest/src/guest.ts:263
invokeAwaiter()sets a timeout whose callback returnsPromise.reject(...), but that rejection is not wired to the returned promise (and can become an unhandled rejection when the timer fires). As written, the timeout never rejects the operation. Implement this as a real timeout (e.g.,Promise.racebetweeninvokeChecker(...)and a promise that rejects in the timer, with properclearTimeoutinfinally), or remove this timer iftimeoutPromisealready enforces the desired limit.
packages/uix-host/src/extensions-provider/extension-registry.ts:149extensionRegistryExtensionsProvider()has an unreachablereturn Promise.resolve({});after already returning thefetchExtensionsFromRegistry(...).then(...)chain. This is dead code and can be removed to avoid confusion (and potential lint warnings about unreachable code).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const depsKey = JSON.stringify(deps); | ||
| const { | ||
| requires, | ||
| provides, | ||
| requires, | ||
| updateOn = "each", | ||
| } = useMemo(() => configFactory(host), baseDeps); | ||
|
|
||
| } = useMemo( | ||
| () => | ||
| host | ||
| ? configFactory(host) | ||
| : ({} as UseExtensionsConfig<Incoming, Outgoing>), | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [host, depsKey], | ||
| ); |
There was a problem hiding this comment.
depsKey is derived via JSON.stringify(deps), which can throw on circular values and also collapses non-serializable deps (e.g., functions become null in arrays). That can cause the hook to stop re-running configFactory when deps actually change, or crash at runtime for certain deps. Prefer using the deps array directly in the dependency list (and require callers to pass stable deps), or compute a safe/stable key that won't throw (e.g., join of primitives) and document the constraint.
| ); | ||
|
|
||
| this.subject.onOutOfScope(fnTicket, cleanup); | ||
| this.receiverTicketCache.set(boundFunction, fnTicket); |
There was a problem hiding this comment.
makeReceiver() looks up receiverTicketCache using the original fn, but stores the ticket using boundFunction as the key. When parent is provided, subsequent visits will call get(fn) again (miss) and create a new ticket each time, which breaks stability/caching of function IDs and can create multiple receivers for the same method. Either always cache by the original fn, or if binding is required, compute the cache key consistently (e.g., look up/store using the same boundFunction value).
| this.receiverTicketCache.set(boundFunction, fnTicket); | |
| this.receiverTicketCache.set(fn, fnTicket); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 77 changed files in this pull request and generated 5 comments.
Files not reviewed (2)
- e2e/all-versions/guest-app/package-lock.json: Language not supported
- e2e/all-versions/host-app/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
packages/uix-host-react/src/hooks/useExtensions.ts:127
boundryExtensionPointsAsStringis rebuilt with.map(...)on every render, so it gets a new array identity each time. BecausegetExtensionsdepends on it, the callback (and the subscribing effect) will also churn/resubscribe even whenextensionPointsdidn't change. Memoize the mapped array (e.g.useMemokeyed byextensionPoints) to keep dependencies stable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const final = setTimeout( | ||
| () => Promise.reject(`${address} doesn't exist`), | ||
| 20000, | ||
| ); | ||
| const res = await this.invokeChecker(invoker, address); | ||
|
|
||
| return new Promise((resolve) => { | ||
| clearTimeout(final); | ||
| return resolve(res); | ||
| }).catch((e) => { | ||
| clearTimeout(final); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return e; | ||
| }); |
| it("BUG: setTimeout callback returns an unhandled rejected Promise instead of rejecting the returned promise", async () => { | ||
| // Intercept setTimeout to capture the 20s timeout callback without | ||
| // triggering a real timer (and without jest fake timers, which prevent | ||
| // Promise microtasks from settling in this test environment). | ||
| const capturedCallbacks: Array<() => unknown> = []; | ||
|
|
||
| jest.spyOn(global, "setTimeout").mockImplementation( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (cb: any, ms?: number) => { | ||
| if (ms === 20000) { | ||
| capturedCallbacks.push(cb); | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| } | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| }, | ||
| ); | ||
|
|
||
| const guest = new Guest({ id: "test-guest" }); | ||
|
|
||
| // invokeChecker never resolves — simulates the host method not existing | ||
| jest | ||
| .spyOn(guest as unknown as GuestPrivate, "invokeChecker") | ||
| .mockReturnValue( | ||
| new Promise(() => { | ||
| /* noop */ | ||
| }), | ||
| ); | ||
|
|
||
| let isSettled = false; | ||
|
|
||
| (guest as unknown as GuestPrivate) | ||
| .invokeAwaiter(jest.fn(), testAddress) | ||
| .then( | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush initial microtasks | ||
|
|
||
| expect(capturedCallbacks).toHaveLength(1); | ||
|
|
||
| // Fire the 20-second timeout callback manually | ||
| const callbackResult = capturedCallbacks[0](); | ||
|
|
||
| // BUG (1): The callback returns a rejected Promise — this is an unhandled | ||
| // rejection. The correct fix would be to reject the invokeAwaiter promise | ||
| // via a mechanism that's connected to the outer async function (e.g. | ||
| // Promise.race or a reject handle captured from the outer promise). | ||
| expect(callbackResult).toBeInstanceOf(Promise); | ||
| // Handle the rejection here so it doesn't become an unhandled rejection | ||
| await expect(callbackResult as Promise<unknown>).rejects.toMatch( | ||
| /doesn't exist/, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush remaining microtasks | ||
|
|
||
| // BUG (2): The awaiterPromise is still pending — the setTimeout callback | ||
| // rejection was never connected to it, so the caller will wait forever. | ||
| expect(isSettled).toBe(false); | ||
| }); |
| const { host } = useHost(); | ||
| const [hostError, setHostError] = useState<Error>(); | ||
| const extensionPoints = useContext(ExtensibleComponentBoundaryContext); |
| const depsKey = JSON.stringify(deps); | ||
| const { | ||
| requires, | ||
| provides, | ||
| requires, | ||
| updateOn = "each", | ||
| } = useMemo(() => configFactory(host), baseDeps); | ||
|
|
||
| } = useMemo( | ||
| () => | ||
| host | ||
| ? configFactory(host) | ||
| : ({} as UseExtensionsConfig<Incoming, Outgoing>), | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [host, depsKey], | ||
| ); |
| if (connection) { | ||
| connection.tunnel.destroy(); | ||
| } | ||
| }; | ||
| }, [guest, host, methods, privateMethods]); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 77 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- e2e/all-versions/guest-app/package-lock.json: Language not supported
- e2e/all-versions/host-app/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const final = setTimeout( | ||
| () => Promise.reject(`${address} doesn't exist`), | ||
| 20000, | ||
| ); | ||
| const res = await this.invokeChecker(invoker, address); | ||
|
|
||
| return new Promise((resolve) => { | ||
| clearTimeout(final); | ||
| return resolve(res); | ||
| }).catch((e) => { | ||
| clearTimeout(final); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return e; | ||
| }); | ||
| } |
| it("BUG: setTimeout callback returns an unhandled rejected Promise instead of rejecting the returned promise", async () => { | ||
| // Intercept setTimeout to capture the 20s timeout callback without | ||
| // triggering a real timer (and without jest fake timers, which prevent | ||
| // Promise microtasks from settling in this test environment). | ||
| const capturedCallbacks: Array<() => unknown> = []; | ||
|
|
||
| jest.spyOn(global, "setTimeout").mockImplementation( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (cb: any, ms?: number) => { | ||
| if (ms === 20000) { | ||
| capturedCallbacks.push(cb); | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| } | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| }, | ||
| ); | ||
|
|
||
| const guest = new Guest({ id: "test-guest" }); | ||
|
|
||
| // invokeChecker never resolves — simulates the host method not existing | ||
| jest | ||
| .spyOn(guest as unknown as GuestPrivate, "invokeChecker") | ||
| .mockReturnValue( | ||
| new Promise(() => { | ||
| /* noop */ | ||
| }), | ||
| ); | ||
|
|
||
| let isSettled = false; | ||
|
|
||
| (guest as unknown as GuestPrivate) | ||
| .invokeAwaiter(jest.fn(), testAddress) | ||
| .then( | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush initial microtasks | ||
|
|
||
| expect(capturedCallbacks).toHaveLength(1); | ||
|
|
||
| // Fire the 20-second timeout callback manually | ||
| const callbackResult = capturedCallbacks[0](); | ||
|
|
||
| // BUG (1): The callback returns a rejected Promise — this is an unhandled | ||
| // rejection. The correct fix would be to reject the invokeAwaiter promise | ||
| // via a mechanism that's connected to the outer async function (e.g. | ||
| // Promise.race or a reject handle captured from the outer promise). | ||
| expect(callbackResult).toBeInstanceOf(Promise); | ||
| // Handle the rejection here so it doesn't become an unhandled rejection | ||
| await expect(callbackResult as Promise<unknown>).rejects.toMatch( | ||
| /doesn't exist/, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush remaining microtasks | ||
|
|
||
| // BUG (2): The awaiterPromise is still pending — the setTimeout callback | ||
| // rejection was never connected to it, so the caller will wait forever. | ||
| expect(isSettled).toBe(false); | ||
| }); |
| if ("exc-module-runtime" in window) { | ||
| metrics.mertricsInstance = createMetricsInstance(); | ||
| return; |
- Revert useEffect deps in Extensible.tsx back to [debug, hostName, runtimeContainer, extensions] to avoid spurious re-runs caused by host state feedback and unintended guestOptions reactivity; suppress the exhaustive-deps lint warning with eslint-disable comment - Revert scope type in ExtensibleWrapper.tsx from Record<string, string> back to Record<string, any> to preserve the public TypeScript contract; suppress no-explicit-any with eslint-disable comment - Restore unload: mockUnload to MockedHost.mockImplementation in Extensible.test.tsx — accidentally dropped by alphabetical key reordering during the linting pass, causing 4 tests to silently fail Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 77 changed files in this pull request and generated 11 comments.
Files not reviewed (2)
- e2e/all-versions/guest-app/package-lock.json: Language not supported
- e2e/all-versions/host-app/package-lock.json: Language not supported
Comments suppressed due to low confidence (3)
packages/uix-host-react/src/components/GuestUIFrame.tsx:207
- This file previously had a
defaultexport (now removed). If any downstream code relies on deep-importingGuestUIFrameas a default export, this is a breaking change. Consider keeping adefaultexport for backwards compatibility (or explicitly documenting the breaking change) while still exporting the namedGuestUIFrame.
packages/uix-host-react/src/components/Extensible.tsx:257 - This component file previously provided a
defaultexport (now removed). If consumers deep-importExtensibleas a default export, this becomes a breaking change. Consider retaining adefaultexport for compatibility (or clearly documenting the breaking change) while keeping the named export.
packages/uix-host-react/src/components/ExtensibleWrapper/ExtensibleWrapper.tsx:154 - This file previously exported
ExtensibleWrapperas adefaultexport (now removed). If any consumers deep-import it via default export, this is a breaking change. Consider keeping adefaultexport for backwards compatibility (or documenting the breaking change) while still exporting the namedExtensibleWrapper.
You can also share your feedback on Copilot code review. Take the survey.
| private async invokeAwaiter( | ||
| invoker: RemoteMethodInvoker<unknown>, | ||
| address: HostMethodAddress<unknown[]>, | ||
| ): Promise<any> { | ||
| const final = setTimeout(() => { | ||
| return new Promise((resolve, reject) => | ||
| reject(`${address} doesn't exist`), | ||
| ); | ||
| }, 20000); | ||
| const final = setTimeout( | ||
| () => Promise.reject(`${address} doesn't exist`), | ||
| 20000, | ||
| ); | ||
| const res = await this.invokeChecker(invoker, address); | ||
|
|
||
| return new Promise((resolve) => { | ||
| clearTimeout(final); | ||
| return resolve(res); | ||
| }).catch((e) => { | ||
| clearTimeout(final); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return e; | ||
| }); |
| it("BUG: setTimeout callback returns an unhandled rejected Promise instead of rejecting the returned promise", async () => { | ||
| // Intercept setTimeout to capture the 20s timeout callback without | ||
| // triggering a real timer (and without jest fake timers, which prevent | ||
| // Promise microtasks from settling in this test environment). | ||
| const capturedCallbacks: Array<() => unknown> = []; | ||
|
|
||
| jest.spyOn(global, "setTimeout").mockImplementation( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (cb: any, ms?: number) => { | ||
| if (ms === 20000) { | ||
| capturedCallbacks.push(cb); | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| } | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| }, | ||
| ); | ||
|
|
||
| const guest = new Guest({ id: "test-guest" }); | ||
|
|
||
| // invokeChecker never resolves — simulates the host method not existing | ||
| jest | ||
| .spyOn(guest as unknown as GuestPrivate, "invokeChecker") | ||
| .mockReturnValue( | ||
| new Promise(() => { | ||
| /* noop */ | ||
| }), | ||
| ); | ||
|
|
||
| let isSettled = false; | ||
|
|
||
| (guest as unknown as GuestPrivate) | ||
| .invokeAwaiter(jest.fn(), testAddress) | ||
| .then( | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush initial microtasks | ||
|
|
||
| expect(capturedCallbacks).toHaveLength(1); | ||
|
|
||
| // Fire the 20-second timeout callback manually | ||
| const callbackResult = capturedCallbacks[0](); | ||
|
|
||
| // BUG (1): The callback returns a rejected Promise — this is an unhandled | ||
| // rejection. The correct fix would be to reject the invokeAwaiter promise | ||
| // via a mechanism that's connected to the outer async function (e.g. | ||
| // Promise.race or a reject handle captured from the outer promise). | ||
| expect(callbackResult).toBeInstanceOf(Promise); | ||
| // Handle the rejection here so it doesn't become an unhandled rejection | ||
| await expect(callbackResult as Promise<unknown>).rejects.toMatch( | ||
| /doesn't exist/, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush remaining microtasks | ||
|
|
||
| // BUG (2): The awaiterPromise is still pending — the setTimeout callback | ||
| // rejection was never connected to it, so the caller will wait forever. | ||
| expect(isSettled).toBe(false); | ||
| }); |
| "lint": "run-p lint:*", | ||
| "lint:eslint": "npm run --workspaces --if-present lint", | ||
| "lint:format": "prettier --check packages/*/src", | ||
| "lint:pkg": "fixpack --dryRun; npx --ws -p fixpack -c 'fixpack --dryRun --quiet'", |
| "del-cli": "^5.0.0", | ||
| "eslint": "^8.21.0", | ||
| "eslint": "9.31.0", | ||
| "eslint-config-prettier": "^10.1.8", | ||
| "eslint-import-resolver-typescript": "^4.4.4", |
| "lint": "run-p lint:*", | ||
| "lint:eslint": "npm run --workspaces --if-present lint", |
packages/uix-core/package.json
Outdated
| "scripts": { | ||
| "build": "tsup", | ||
| "build:esm": "tsup --format esm,cjs", | ||
| "lint": "eslint src --fix", |
| "scripts": { | ||
| "build": "tsup", | ||
| "build:esm": "tsup --format esm,cjs,iife", | ||
| "lint": "eslint src --fix", | ||
| "watch": "tsup --watch --silent" |
| "scripts": { | ||
| "build": "tsup", | ||
| "build:esm": "tsup --format esm,cjs", | ||
| "lint": "eslint src --fix", | ||
| "test": "NODE_ENV=test jest", |
| public get metadata(): GuestMetadata { | ||
| if (this.isReady() && this.guestServer) { | ||
| const server = this.guestServer.getRemoteApi(); | ||
|
|
||
| return server && server.metadata; | ||
| } | ||
| } |
| }, {}), | ||
| ); | ||
|
|
||
| return Promise.resolve({}); | ||
| } | ||
| }; |
…config This branch upgrades ESLint v8 → v9.31.0 and introduces a flat config setup (eslint.base.mjs shared factory + per-package eslint.config.mjs), wires ESLint into npm run lint via lint:eslint, and restores uix-guest as a Jest project in jest.config.ts. Update copilot-instructions.md: - Stack line: ESLint 9 flat config, Prettier 3 - Step 5 in command sequence: lint now includes ESLint --fix - npm test description: lint phase now includes ESLint - ESLint section: rewrite for flat config; document key rules that cause failures (import order, key order, function length, etc.) - Testing config: 4 Jest projects (uix-guest restored) - Key config table: replace .eslintrc.cjs with eslint.base.mjs Update CLAUDE.md: - Clarify npm run lint and npm run format descriptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Update .nvmrc from 16.* to 18.* — ESLint 9 requires Node >= 18
- Remove unreachable 'return Promise.resolve({})' in
extensionRegistryExtensionsProvider() (dead code after a return)
- Fix typo: metrics.mertricsInstance -> metrics.metricsInstance
(getter, setter, and call site in runtimeSpy — internal only)
- Change package lint scripts from 'eslint src --fix' to 'eslint src'
so npm run lint / npm test does not auto-mutate the working tree;
auto-fix is handled separately via npm run format
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renaming the public getter/setter from mertricsInstance to metricsInstance caused a TS2300 duplicate identifier clash with the existing private backing field of the same name. Rename the private field to _metricsInstance to distinguish it from the public accessor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 80 changed files in this pull request and generated 9 comments.
Files not reviewed (2)
- e2e/all-versions/guest-app/package-lock.json: Language not supported
- e2e/all-versions/host-app/package-lock.json: Language not supported
You can also share your feedback on Copilot code review. Take the survey.
| private async invokeAwaiter( | ||
| invoker: RemoteMethodInvoker<unknown>, | ||
| address: HostMethodAddress<unknown[]>, | ||
| ): Promise<any> { | ||
| const final = setTimeout(() => { | ||
| return new Promise((resolve, reject) => | ||
| reject(`${address} doesn't exist`), | ||
| ); | ||
| }, 20000); | ||
| const final = setTimeout( | ||
| () => Promise.reject(`${address} doesn't exist`), | ||
| 20000, | ||
| ); | ||
| const res = await this.invokeChecker(invoker, address); | ||
|
|
||
| return new Promise((resolve) => { | ||
| clearTimeout(final); | ||
| return resolve(res); | ||
| }).catch((e) => { | ||
| clearTimeout(final); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return e; | ||
| }); |
| it("BUG: setTimeout callback returns an unhandled rejected Promise instead of rejecting the returned promise", async () => { | ||
| // Intercept setTimeout to capture the 20s timeout callback without | ||
| // triggering a real timer (and without jest fake timers, which prevent | ||
| // Promise microtasks from settling in this test environment). | ||
| const capturedCallbacks: Array<() => unknown> = []; | ||
|
|
||
| jest.spyOn(global, "setTimeout").mockImplementation( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (cb: any, ms?: number) => { | ||
| if (ms === 20000) { | ||
| capturedCallbacks.push(cb); | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| } | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| }, | ||
| ); | ||
|
|
||
| const guest = new Guest({ id: "test-guest" }); | ||
|
|
||
| // invokeChecker never resolves — simulates the host method not existing | ||
| jest | ||
| .spyOn(guest as unknown as GuestPrivate, "invokeChecker") | ||
| .mockReturnValue( | ||
| new Promise(() => { | ||
| /* noop */ | ||
| }), | ||
| ); | ||
|
|
||
| let isSettled = false; | ||
|
|
||
| (guest as unknown as GuestPrivate) | ||
| .invokeAwaiter(jest.fn(), testAddress) | ||
| .then( | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush initial microtasks | ||
|
|
||
| expect(capturedCallbacks).toHaveLength(1); | ||
|
|
||
| // Fire the 20-second timeout callback manually | ||
| const callbackResult = capturedCallbacks[0](); | ||
|
|
||
| // BUG (1): The callback returns a rejected Promise — this is an unhandled | ||
| // rejection. The correct fix would be to reject the invokeAwaiter promise | ||
| // via a mechanism that's connected to the outer async function (e.g. | ||
| // Promise.race or a reject handle captured from the outer promise). | ||
| expect(callbackResult).toBeInstanceOf(Promise); | ||
| // Handle the rejection here so it doesn't become an unhandled rejection | ||
| await expect(callbackResult as Promise<unknown>).rejects.toMatch( | ||
| /doesn't exist/, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush remaining microtasks | ||
|
|
||
| // BUG (2): The awaiterPromise is still pending — the setTimeout callback | ||
| // rejection was never connected to it, so the caller will wait forever. | ||
| expect(isSettled).toBe(false); | ||
| }); |
| import { createConfig } from "../../eslint.base.mjs"; | ||
|
|
||
| export default createConfig({ tsconfigRootDir: import.meta.dirname }); |
| import { createConfig } from "../../eslint.base.mjs"; | ||
|
|
||
| export default createConfig({ | ||
| tsconfigRootDir: import.meta.dirname, | ||
| includeReact: true, | ||
| }); |
| import { createConfig } from "../../eslint.base.mjs"; | ||
|
|
||
| export default createConfig({ tsconfigRootDir: import.meta.dirname }); |
| import { createConfig } from "../../eslint.base.mjs"; | ||
|
|
||
| export default createConfig({ tsconfigRootDir: import.meta.dirname }); |
| @@ -4,6 +4,9 @@ const sdkProject = (sdkName: string, overrides: JestConfigWithTsJest) => ({ | |||
| displayName: `uix-${sdkName}`, | |||
| testMatch: [`<rootDir>/packages/uix-${sdkName}/src/**/*.test.ts`], | |||
CLAUDE.md
Outdated
| npm run test:unit:watch # Watch mode for unit tests | ||
| npm run lint # Check formatting and linting | ||
| npm run format # Auto-fix formatting issues | ||
| npm run lint # ESLint (--fix), Prettier check, and fixpack — all in parallel |
.github/copilot-instructions.md
Outdated
| @@ -48,22 +48,31 @@ npm run format # Runs `format:code` (Prettier --write) + `format:pkg` (fi | |||
| npm run declarations:build | |||
| ``` | |||
|
|
|||
| `npm test` runs `lint → test:unit → test:subtests` sequentially via `run-s`. The `lint` step only runs the Prettier check and fixpack; it does not run ESLint. In workflows that invoke `npm test` (and in local pre-publish checks), all three must pass; do not skip the `lint` phase. | |||
| `npm test` runs `lint → test:unit → test:subtests` sequentially via `run-s`. The `lint` step runs ESLint (with `--fix`), Prettier check, and fixpack in parallel. All three phases must pass; do not skip the `lint` phase. | |||
|
|
|||
| **Production build** (used in CI release): `npm run build:production` | |||
|
|
|||
| ## ESLint | |||
|
|
|||
| ESLint is configured via `.eslintrc.cjs` at the root (ESLint v8). It extends `eslint:recommended`, `plugin:@typescript-eslint/recommended`, and `plugin:@typescript-eslint/recommended-requiring-type-checking`. Two unsafe-assignment/unsafe-return rules are turned off; all other recommended TypeScript rules apply. | |||
| ESLint 9 flat config. The shared factory is in `eslint.base.mjs` at the repo root; each package imports it in its own `eslint.config.mjs`. `npm run lint` runs `lint:eslint` (per-package `eslint src --fix`) in parallel with Prettier and fixpack. | |||
|
|
|||
…pace scripts npm run lint now runs ESLint in check-only mode (eslint src, no --fix). Auto-fixing ESLint violations requires running eslint src --fix manually inside the relevant package. Update CLAUDE.md and copilot-instructions.md to reflect this accurately. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove four high-churn, opinionated rules from eslint.base.mjs: - Remove sort-keys-fix entirely (alphabetic key ordering is unusual and breaks intentional semantic groupings) - Simplify padding-line-between-statements to only the import separator rule (remove 9 sub-rules enforcing blank lines around all statements) - Turn off func-style and arrow-body-style (style preferences with no correctness impact; function declarations have legitimate hoisting uses) - Turn off max-statements (max-lines-per-function: 75 already limits function size more meaningfully) Also remove now-unused eslint-disable-next-line max-statements comments from 7 src files that were suppressing the removed rule. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 80 changed files in this pull request and generated 8 comments.
Files not reviewed (2)
- e2e/all-versions/guest-app/package-lock.json: Language not supported
- e2e/all-versions/host-app/package-lock.json: Language not supported
You can also share your feedback on Copilot code review. Take the survey.
| const final = setTimeout( | ||
| () => Promise.reject(`${address} doesn't exist`), | ||
| 20000, | ||
| ); | ||
| const res = await this.invokeChecker(invoker, address); | ||
|
|
||
| return new Promise((resolve) => { | ||
| clearTimeout(final); | ||
| return resolve(res); | ||
| }).catch((e) => { | ||
| clearTimeout(final); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return e; | ||
| }); |
| const keys = Object.keys(item); | ||
| const hasRoot = keys.includes(NS_ROOT); | ||
|
|
||
| if (hasRoot && keys.length != 1) { |
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| type ExtractKeys<T, U> = { | ||
| [P in keyof T]: T[P] extends U ? P : never; | ||
| }[keyof T]; |
| const { host, error } = useHost(); | ||
|
|
||
| if (error) { | ||
| return { | ||
| error, | ||
| extensions: NO_EXTENSIONS, | ||
| loading: false, | ||
| error, | ||
| }; | ||
| } | ||
|
|
||
| /* eslint-disable react-hooks/rules-of-hooks, react-hooks/exhaustive-deps -- early return above is pre-existing; fixing requires a larger refactor */ | ||
| const [hostError, setHostError] = useState<Error>(); | ||
| const extensionPoints = useContext(ExtensibleComponentBoundaryContext); | ||
| const boundryExtensionPointsAsString = extensionPoints?.map( |
| const ref = useRef<HTMLIFrameElement>(); | ||
| const { host } = useHost(); | ||
|
|
||
| if (!host) { | ||
| return null; | ||
| } | ||
|
|
||
| const guest = host.guests.get(guestId); | ||
| const frameUrl = new URL(src, guest.url.href); | ||
|
|
||
| /* eslint-disable react-hooks/rules-of-hooks, react-hooks/exhaustive-deps -- early return above is pre-existing; fixing requires a larger refactor */ | ||
| useEffect(() => { | ||
| if (ref.current) { |
| const guest = host.guests.get(guestId); | ||
| const frameUrl = new URL(src, guest.url.href); | ||
|
|
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| export class Port<_GuestApi = unknown> | ||
| extends Emitter<GuestConnectionEvents> | ||
| implements GuestConnection |
| it("BUG: setTimeout callback returns an unhandled rejected Promise instead of rejecting the returned promise", async () => { | ||
| // Intercept setTimeout to capture the 20s timeout callback without | ||
| // triggering a real timer (and without jest fake timers, which prevent | ||
| // Promise microtasks from settling in this test environment). | ||
| const capturedCallbacks: Array<() => unknown> = []; | ||
|
|
||
| jest.spyOn(global, "setTimeout").mockImplementation( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (cb: any, ms?: number) => { | ||
| if (ms === 20000) { | ||
| capturedCallbacks.push(cb); | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| } | ||
|
|
||
| return 0 as unknown as ReturnType<typeof setTimeout>; | ||
| }, | ||
| ); | ||
|
|
||
| const guest = new Guest({ id: "test-guest" }); | ||
|
|
||
| // invokeChecker never resolves — simulates the host method not existing | ||
| jest | ||
| .spyOn(guest as unknown as GuestPrivate, "invokeChecker") | ||
| .mockReturnValue( | ||
| new Promise(() => { | ||
| /* noop */ | ||
| }), | ||
| ); | ||
|
|
||
| let isSettled = false; | ||
|
|
||
| (guest as unknown as GuestPrivate) | ||
| .invokeAwaiter(jest.fn(), testAddress) | ||
| .then( | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| () => { | ||
| isSettled = true; | ||
| }, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush initial microtasks | ||
|
|
||
| expect(capturedCallbacks).toHaveLength(1); | ||
|
|
||
| // Fire the 20-second timeout callback manually | ||
| const callbackResult = capturedCallbacks[0](); | ||
|
|
||
| // BUG (1): The callback returns a rejected Promise — this is an unhandled | ||
| // rejection. The correct fix would be to reject the invokeAwaiter promise | ||
| // via a mechanism that's connected to the outer async function (e.g. | ||
| // Promise.race or a reject handle captured from the outer promise). | ||
| expect(callbackResult).toBeInstanceOf(Promise); | ||
| // Handle the rejection here so it doesn't become an unhandled rejection | ||
| await expect(callbackResult as Promise<unknown>).rejects.toMatch( | ||
| /doesn't exist/, | ||
| ); | ||
|
|
||
| await Promise.resolve(); // flush remaining microtasks | ||
|
|
||
| // BUG (2): The awaiterPromise is still pending — the setTimeout callback | ||
| // rejection was never connected to it, so the caller will wait forever. | ||
| expect(isSettled).toBe(false); | ||
| }); |
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: