fix: capture lynx.getDevtool() once to avoid errors on page close#521
Closed
upupming wants to merge 16 commits into
Closed
fix: capture lynx.getDevtool() once to avoid errors on page close#521upupming wants to merge 16 commits into
upupming wants to merge 16 commits into
Conversation
…er (#2) The `root-order-page` listener was the only call site in `createAdapter` still reading the bare global `document` instead of going through the `window = preactDevtoolsCtx` alias used by every other DOM access in this file. In browser shells `globalThis.document` and `preactDevtoolsCtx.document` point at the same object so this worked by accident. In the ReactLynx host `globalThis.document` is undefined and `react-lynx/setup.ts` shims `preactDevtoolsCtx.document` with the background root snapshot, so any `refresh` from the panel crashed with: TypeError: cannot read property 'body' of undefined at sendToDevtools at applyFilters (renderer.ts:275) at refresh (renderer.ts:215) Route the access through `window.document.body` so the ReactLynx shim is honoured. Add a regression test under `src/adapter/adapter/` that exercises the `root-order-page` handler with `globalThis.document` deliberately removed to lock in the fix.
Reference internal-preact's release workflow: - Trigger publish on push to main (keep manual workflow_dispatch) - Insert a UTC timestamp before the commit sha in the published version so prerelease ordering stays monotonic across commits
# Conflicts: # .gitignore # package-lock.json # package.json # src/adapter/10/bindings.ts # src/adapter/adapter/port.ts # src/adapter/protocol/events.ts # test-e2e/tests/profiler/flamegraph/profiler-static-subtree.test.ts # tools/build-plugins/esbuild-plugins.mjs # tools/test-setup.js # tsconfig.json
…t to 0.121.0 (#4) * fix: broken hooks detection (preactjs#507) Note that hooks are pretty much supported everywhere so this had no production impact. * fix: incorrect props string truncation (preactjs#508) * chore: only fetch required preact versions for tests (preactjs#509) * chore: harden e2e tests (preactjs#510) Lots of flakyness was caused by trying to one-shot assertions by grabbing an element and asserting on it instead of polling. Other issues were basically race conditions between elements of which there were plenty. All of this is now much more reliable thanks to new assertions in playwright. * fix: potential memory leak due to growing string table (preactjs#511) The string table was unbounded, so add a LRU cache to limit growth. Also make encoding of strings faster while we're at it. * fix: custom filter not removed + use popover (preactjs#512) * chore: move unit tests to vitest (preactjs#513) * chore: switch to oxlint + oxfmt (preactjs#514) * chore: remove husky + lint-staged (preactjs#515) * chore: update dependencies (preactjs#516) Also fix text signal issue with newer signals versions * chore: fix inline build failing (preactjs#517) * chore: update dependencies (preactjs#518) * fix: make hook inspection attachment more robust (preactjs#519) Using a `setTimeout` was always a bit meh. --------- Co-authored-by: Marvin Hagemeister <hello@marvinh.dev>
…tes roots `_root` decided whether the user-passed container is a real DOM node or a virtual islands container by checking `"Node" in globalThis && parent instanceof preactDevtoolsCtx.Node`. The original code used `parent instanceof Node`; the ReactLynx fork swapped the right hand side to `preactDevtoolsCtx.Node` (a shim of `BackgroundSnapshotInstance`) but kept `"Node" in globalThis` as the guard. In the ReactLynx Background VM `globalThis.Node` does not exist, so the AND short-circuits and `treeParent` falls through to `parent.parentNode`. The application root (`__root`) has no parent, so `userRootToContainer` stored `null`. `_commit` then skipped `roots.set(vnode, dom)` because of the `if (dom)` guard, leaving `getRootMappings()` permanently empty. User-visible effect: panel-side `refresh` (`adapter.ts:195` -> `renderer.applyFilters`) iterated zero roots and emitted no `operation_v2`, so a reconnecting panel could not recover the tree without a full `Page.reload`. The browser content-script even contains a comment explaining this is the path it relies on: `shells/shared/content-script.ts:30-36`. Use the same shim on both sides of the guard. Behavior is unchanged on browser hosts, where `preactDevtoolsCtx.Node === window.Node`, and unchanged in test fixtures that do not set `preactDevtoolsCtx.Node`. Add `src/adapter/10/options.test.tsx` that simulates the ReactLynx VM (no `globalThis.Node`, parentless container, `preactDevtoolsCtx.Node` set to a host-specific class) and asserts the renderer's `roots` map gets the container after a `_commit`. Verified the test fails on `main` (`expected 0 to equal 1`) and passes with the fix.
…omium) The Lynx DevTool host embeds Chromium 108 (Electron 22), which predates the HTML Popover API (Chromium 114). preactjs#512 moved the element/profiler filter dropdowns to `popover="auto"` + `popovertarget`, so on that host they render visible by default and cannot be dismissed. Revert just the popover mechanism (restore OutsideClick + `filterVisible` show/hide); keep preactjs#512's id-based custom-filter handling.
fix: avoid native Popover API for filter dropdowns (broken on old Chromium)
When the devtools panel opened with no tree (it connected after the page rendered, or reopened without its state), it reloaded the whole page (`Page.reload`), losing app state. Instead emit a `refresh` so the in-app hook re-walks its existing roots and re-sends the tree, with no reload. The Lynx bridge has no content-script-style buffer, so the panel also missed the initial `attach` and never learned `supportsHooks`; re-send `attach` for each renderer on `refresh` (idempotent) so hooks inspection works too. If there are genuinely no roots, an empty tree is correct — reloading is left to the user.
…f-reload fix: recover the tree via refresh instead of reloading the page
…dev (#8) Setup was gated on `__DEV__`, so it never ran in production. With `@lynx-js/react-alias-rsbuild-plugin` stripping `@lynx-js/preact-devtools` in production unless `REACT_DEVTOOL` is set, the import itself is the opt-in: if this module runs at all, devtools is wanted. Drop the `__DEV__` guard so a user-imported devtools also works in production (paired with the lynx-stack `REACT_DEVTOOL` toolchain changes). The test environment is still skipped.
Bring the demo's Lynx dependencies up to current: - @lynx-js/rspeedy 0.10.6 -> 0.15.0 - @lynx-js/react-rsbuild-plugin 0.10.11 -> 0.17.0 - @lynx-js/react 0.112.2 -> 0.121.2 - @lynx-js/qrcode-rsbuild-plugin 0.4.0 -> 0.5.0 - @lynx-js/types 3.3.0 -> 3.7.0
- @lynx-js/react 0.121.0 -> 0.121.2 - @lynx-js/types ^3.3.0 -> ^4.0.0 - @lynx-js/testing-environment 0.1.5 -> 0.3.0 Adapt the lynx test setup to `@lynx-js/testing-environment` >=0.2, which takes the host `window` via the constructor instead of reading `global.jsdom`. type-check, vitest, and vitest:lynx all pass.
lynx.getDevtool() can return undefined later in the lifecycle (e.g. when the page is being closed), so calling .dispatchEvent / .addEventListener on the result throws. Capture the devtool instance once after the type check and reuse it.
Contributor
Author
|
Opened against the wrong repository by mistake; reopening on lynx-family/preact-devtools. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
lynx.getDevtool().dispatchEvent(...)throws when the page is being closed, because at that pointlynx.getDevtool()returnsundefinedand accessing.dispatchEventonundefinedfails.This affects both the
postMessagepath (App → Devtools) and theaddEventListenerregistration.Fix
Capture the devtool instance once, right after the existing
typeof lynx.getDevtool !== "function"check, and reuse that reference fordispatchEvent/addEventListenerinstead of callinglynx.getDevtool()again at runtime.Since the reference is captured during setup (when it is guaranteed valid), later calls no longer depend on
lynx.getDevtool()still returning a valid object.Verification
oxfmt --check/oxlint: no new errors (only pre-existingconsolewarnings)tsc --noEmit: no errors for this file