Repository navigation
Conversation
getFocus returns the same value or issue as getResult, together with the path from the root. A failure also carries the path to, and the input of, the step that failed. Traversals report each element's source index. key/optionalKey/at add a Key segment, tag adds a Tag segment, and the some/none/success/failure prisms add Tag (+ Key) segments. Checks, refinements, pick and omit keep the location. make* optics and entries() add an Opaque segment. forEach is now a step of the optic instead of an opaque wrapper around the chain before it, so the walker can see through it.
Member
Author
Contributor
|
Thanks! I've queued a takeover of this PR in our tracker (EFF-3257). The team will review the |
Drop the rule that redirected keys of a traversal's collected array to the element's source: any step into that array now adds an Opaque segment. Built-in steps carry their segments explicitly, with [Opaque] for make* optics and entries(), so the walker no longer special-cases undefined. Segment.Tag.tag is now a string.
🦋 Changeset detectedLatest commit: 755b1a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 33 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
The walker now carries a tree of locations, so an element collected by a nested traversal keeps its own element locations and a step into it is reported as Opaque instead of an invented source index. Segment.Tag.tag is SchemaAST.LiteralValue again, matching what .tag() accepts.
Every non-traversal step now carries its own path segments, so the walker runs one get branch and one segment loop instead of a switch per node kind. The location tree stays: it is the uniform model that keeps nested traversals sound.
Name the success and failure models Diagnostics and DiagnosticsFailure, and update examples and tests to use the diagnostic reader name.
Read, update, and merge PathNode key segments directly instead of retaining a duplicate PropertyKey path.
Expose a shared Location model so nested traversal indices remain available to reporters. Include the current element mappings when a later read fails, and cover both cases with regression tests.
Use common sequencing, failure propagation, and traversal collection while skipping diagnostic allocation on ordinary reads. Cover nested replacements and ensure each traversal element is read once per evaluation.
Clarify traversal index segments, update the consumer release note to getWithDiagnostics, and align the new API versions with the minor-release policy for public additions.
Skip the final EachNode getter during setter compilation. Its setter already finds the matching elements, so collecting the focused values first adds an unused traversal. Preserve reads needed by subsequent optic steps.
This branch has not been deployed
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
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.
Why
We're designing a test runner for Effect. It isn't published yet; this PR prepares one piece of core it will need. In its design, a failed assertion carries data, not a rendered diff: a list of mismatches, each with its path from the root of the value under test, a code, and the expected and actual values. Reporters render that list for humans, write it to JSON or JUnit, or hand it to an agent. The path is how a reader, human or agent, finds the bad value without re-reading the whole object.
One of the planned matchers runs a matcher on the part of a value an optic focuses on. Illustrative, since the API isn't final:
Like every other matcher,
focusshould report mismatches at their real path from the root:["orders"][0]["status"]<Shipped>["trackingId"]. Witheffect/Opticas it is, it can't, because an optic doesn't say where it looks or where it stopped:getResultreturns the failing step's issue unchanged, relative to that step:tagfails with a bareInvalidValueandat(5)fails withPointer([5], MissingKey). Neither knows the steps before it.{ _tag: "Pending" }".kin that array isn't indexkin the source, so a mismatch atfocus[0]can't be traced back toorders[1].toString.The best a matcher can do today is report relative to the focus, under a fixed label:
focus[0]: expected "A1", got "B2". The reader can't tell which order that is. Our prototype does exactly that.Nothing in this PR depends on the runner.
getFocusis useful to anything that reports on optics, such as form validation errors or logging which field was updated.Requirement
What the planned
focusmatcher needs from core. Please weigh other designs against this list, not just against ours:focus[k]maps back to the real element.tagsteps, since they say which variant was expected.getResultreturns. Existing callers may depend on it.Summary
One new function next to
getResult. It walks the same steps and records where it goes:some/success/failureadd aTagand then aKey(<Some>["value"]);noneadds<None>.refine,notUndefined,pickandomitkeep the location and add nothing.key(0)afterforEach) addsOpaque; useelementsfor each element's source path.forEachused to wrap the whole chain in front of it into one opaquemakeOptional. It's now a step of the optic, so the walker can see through it. Its get/set behaviour is unchanged.What the planned
focusmatcher would do with it:Alternatives considered
optic.nodein the matcher libraryat/tag/optionalKeykeys live only in closures; a traversal's indices still need re-walkingoptic.path(what the research first recommended)at("orders")failing would still report["orders"][0]["status"]), can't give a traversal's real indices, and has no input for "got X"getResultfailures root-relative (wrap them inPointer(prefix, issue))getResult's output (requirement 5);Pointerpaths arePropertyKey[], sotagsteps can't appear (requirement 4); no input and no traversal indicesfocus.key("a").index(0))OpticOpen to better ideas, for example making
make*accept the segments a custom optic takes, so it isn'tOpaque.Evidence
Run against
{ orders: [{ status: Pending }, { status: Shipped "A1" }, { status: Shipped "B2" }] }. Paths are joined for readability:getFocustests inpackages/effect/test/Optic.test.tscheck that each failure's issue matchesgetResult's.forEachreplace and arity cases.pnpm check,pnpm jsdocs --checkand the doctests pass.pnpm lint-fixwasn't run; the touched files were formatted with dprint and linted with oxlint directly.Merge Danger
Door: two-way
getFocus,Segment,FocusandFocusFailureare new and tagged@stability experimental, so their shape can still change in a patch once the test runner's matchers use them.getResult,replaceandmodifyreturn exactly what they did before.Blast Radius:
forEachforEachis now built differently inside the module: a step of the composed optic rather than a wrapper around it. The existing tests cover its get, replace, arity-mismatch and inner-failure behaviour.pick/omitand the built-in prisms now get their path metadata internally instead of going through the publicmakeLens/makePrism; their behaviour is the same.Closes EFF-3257