-
Notifications
You must be signed in to change notification settings - Fork 0
feat(fork): a selection that can't be traced to code goes read-only #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,19 @@ export function normalizeNativeSource(value: unknown): string | null { | |
| * selector-only forever); the preload's short-TTL null cache bounds the retry cost. */ | ||
| const attempts = new WeakMap<TaggedElement, Promise<boolean>>(); | ||
|
|
||
| /** Elements whose attempt settled without producing a tag. Snapshots read this to tell | ||
| * "no attempt has finished" (pending — stay editable) apart from "an attempt finished | ||
| * and found nothing" (with the attributes also absent, the element is anonymous and the | ||
| * panel disables editing). Membership is never the whole answer: a tag or a | ||
| * component/file attribute — including one a LATER retry writes — always wins at read | ||
| * time, so stale membership is harmless. */ | ||
| const settledUntagged = new WeakSet<TaggedElement>(); | ||
|
|
||
| /** Whether a native-source attempt for `el` has settled without tagging it. */ | ||
| export function hasSettledUntagged(el: TaggedElement): boolean { | ||
| return settledUntagged.has(el); | ||
| } | ||
|
|
||
| /** The elements a send or selection actually names: each element itself plus the parent | ||
| * and adjacent siblings the structural asks (move/absolute) reference. One helper for | ||
| * BOTH the send barrier and selection promotion so the two fan-outs never drift. */ | ||
|
|
@@ -138,7 +151,15 @@ export function resolveAndTag(el: TaggedElement): Promise<boolean> { | |
| const cached = attempts.get(el); | ||
| if (cached) return cached; | ||
| const resolver = getResolver(); | ||
| if (!resolver) return Promise.resolve(false); | ||
| if (!resolver) { | ||
| // A host with no resolver installed can never address this element — that IS a | ||
| // settled answer, and recording it here is what lets the panel's per-element gate | ||
| // work without a separate page-level concept (PR #72 review). If a resolver appears | ||
| // later (never in practice — preloads install before page scripts), the ordinary | ||
| // retry path still runs because nothing was cached in `attempts`. | ||
| settledUntagged.add(el); | ||
| return Promise.resolve(false); | ||
| } | ||
| const attempt = (async () => { | ||
| let raw: unknown; | ||
| try { | ||
|
|
@@ -175,6 +196,7 @@ export function resolveAndTag(el: TaggedElement): Promise<boolean> { | |
| })(); | ||
| attempts.set(el, attempt); | ||
| void attempt.then((tagged) => { | ||
| if (!tagged) settledUntagged.add(el); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this boolean is the wrong type for the new policy. If |
||
| if (!tagged && attempts.get(el) === attempt) attempts.delete(el); | ||
| }); | ||
| return attempt; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,27 @@ | ||
| import { | ||
| DESIGN_MODE_STYLE_KEYS, | ||
| type DesignModeElementSnapshot, | ||
| type DesignModeSourceState, | ||
| type DesignModeStyleKey, | ||
| } from "../protocol"; | ||
| import { alignCapsFor } from "./align"; | ||
| import { COMPONENT_NAME_ATTR, hasSettledUntagged, SOURCE_FILE_ATTR } from "./nativeSource"; | ||
| import { readSizeModes } from "./sizeMode"; | ||
| import type { DraftStore } from "./vendor/drafts"; | ||
| import { positionStateOf, POSITION_ROWS } from "./vendor/panel-specs"; | ||
| import { basename, parseSourceAttr, type TaggedElement } from "./vendor/source"; | ||
|
|
||
| /** What addressing the request could carry for this element, read live off the DOM plus | ||
| * the attempt ledger. A component name or source file counts as resolved — "Rendered by | ||
| * <X> in file" is real context the agent can act on (PR #67) — so only an element that | ||
| * settled with NONE of the three reads as anonymous. */ | ||
| function sourceStateOf(el: TaggedElement, hasTag: boolean): DesignModeSourceState { | ||
| if (hasTag || el.hasAttribute(COMPONENT_NAME_ATTR) || el.hasAttribute(SOURCE_FILE_ATTR)) { | ||
| return "resolved"; | ||
| } | ||
| return hasSettledUntagged(el) ? "anonymous" : "pending"; | ||
| } | ||
|
Comment on lines
+18
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this re-derives addressing from a side channel + live DOM attrs instead of consuming what resolution already knew. That is the smell that Once
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the resolveAndTag thread for the split decision: the no-resolver hole is fixed at the source, but the attr reads stay — they are the cross-injection persistence layer, not a side channel, and sourceStateOf reading them live is what lets a later retry's component/file attributes upgrade an element without any bookkeeping. |
||
|
|
||
| /** The X/Y readout, in the margin-edge basis the panel's fields also WRITE (POSITION_ROWS | ||
| * owns both halves, so the field can never display a basis it doesn't commit to). */ | ||
| function readOffsets(el: TaggedElement): { x: number; y: number } { | ||
|
|
@@ -40,6 +53,7 @@ export function buildElementSnapshot( | |
| id, | ||
| tag: el.tagName.toLowerCase(), | ||
| sourceLabel: parsed ? `${basename(parsed.file)}:${parsed.line}` : null, | ||
| sourceState: sourceStateOf(el, dcSource !== ""), | ||
| styles, | ||
| sizeModes: readSizeModes(el, drafts), | ||
| offsets: readOffsets(el), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think there's a code-judo move here that makes this much simpler. can we reframe this so these branches disappear?
settledUntaggedexists becauseresolveAndTagreturns a boolean that means "tagged", not "addressable" — component/file-only writes attrs and still returnsfalse, then this set is populated, thensourceStateOfre-reads the attrs to override the set. Three places holding one fact.Worse: the
!resolverpath (return Promise.resolve(false)beforeattempts.set) never reaches the.thenthat fills this set, which is the only reasonForkDesignPanelORs in page-levelsourceMode.Prefer: typed settle result from
resolveAndTag(tagged | context | anonymous | unavailable). Snapshots consume that. Drop the WeakSet. No-resolver settles asunavailable/anonymousimmediately so the panel gate does not need a second concept.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Half taken. The no-resolver settle now happens at the early exit (b1acec5), which removes the panel's second concept — that was the load-bearing part of this finding. Declining the typed-settle-enum-replaces-the-WeakSet refactor: the attribute reads are not a redundant third copy of the fact, they are the persistence layer. Engine module state (any WeakSet or WeakMap) dies on destroy/re-inject while the DOM and its attributes survive — the same reason NATIVE_SOURCE_MARKER_ATTR is an attribute rather than a WeakSet (documented at its declaration). A typed settle result would still have to re-read attributes after re-injection, so it adds a shape without removing a reader.