fix(loader): persist entry config only after a committed update - #77
Open
tdwhere123 wants to merge 2 commits into
Open
fix(loader): persist entry config only after a committed update#77tdwhere123 wants to merge 2 commits into
tdwhere123 wants to merge 2 commits into
Conversation
Write the loader tree after the internal/update chain commits so a veto cannot leave persistence ahead of the live fiber.
YAML group and config identity were not commit. Snapshot the previous fiber.config, require subgroup.data === config for Group, and skip FAILED apply.
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.
Frame
Cordis today is a declarative lifecycle + dependency-graph kernel with cooperative composability: plugins declare effects, and correctness of unload depends on those plugins (or the host APIs they used) supplying inverses. Host-mediated paths (
ctx.timeout/ctx.interval,ctx.on,ctx.provide) already invert themselves. BaresetInterval/ sockets / files do not. This PR does not claim that unload clears those, and it does not turn Core into a capability or resource-ownership kernel.That split is the useful one for later work:
Desired is the payload someone asked for (
fiber.update(config)). Committed is the payload the runtime actually accepted (inner assigned a newfiber.config, or Group assignedsubgroup.data). Persistence is whattree.write()will later round-trip as the graph.Today those three are implicit. Inner assigns
fiber.configthenrestart()(packages/core/src/fiber.ts). Persistence used to run beforenext(), so a veto could write Desired while runtime stayed on the old object. This PR makes one local invariant true:Cite this PR for later inner-layer work (Include same-path intercept, HMR mixed generation, a first-class Reconcile / Desired≠Committed model). Do not cite it as effect-ownership being closed — that remains disposer tracking, not resource ownership.
Problem
The persist hook on
internal/updatewroteentry.options.configand calledtree.write()beforenext(). An empty fiber-local hook is a waterfall veto. Runtime then kept the old config; disk/in-memory persistence already had the new one.Group is a second, real commit path:
@cordisjs/plugin-groupswallowsnext()and runsEntryGroup.update. That is not a veto, so Group still has to persist — but the YAMLgroup: trueflag is not “Group intercept ran.”cordisjs/cordis#19(childctx.plugin()+dispose()must not disable the parent entry) already holds on currentmainviainternal/plugincase 3. This PR only locks it; it does not retouch that hook (#66occupies the later inject-meta edit).Change
Persist after
next()(and after a thenable settles). Then write only if the payload was accepted:this.config !== previous, wherepreviousis snapshotted beforenext()(inner assigned a new reference)entry.subgroup?.data === config(EntryGroup.updateassignsthis.data = configsynchronously) — not YAMLgroup: truethis.state === FiberState.FAILEDskips the write (inner may already have assignedfiber.config)noSaveand child-fiber updates still skip the writerSame-path Include intercept also swallows
next()and has no Group signal. This PR treats that as non-commit and does not edit Include (#47/#57/#74already occupy that file). Old persist-before-next()wrote Include plugin config even when Include ignored the new object; stopping that write is intentional.Tests:
packages/loader/tests/persist-order.spec.ts. MockLoaderwrite()snapshots withstructuredCloneand counts calls, soloader.datais not an alias of liveroot.data. Emptyinternal/updatehooks inindex.spec.tsnow callnext()(an empty hook is a veto; the old self-update tests were documenting persist-before-veto).Test plan
yarn test loader— ordinary commit, local/global/async veto, Group with and without YAMLgroup, Group-shaped intercept veto, same-reference veto, FAILED restart, delayednext(), same-turnfiber.updatedoes not write yet,noSave, child dispose (Loader 将子插件 dispose 视为整个插件 dispose #19 lock)group: true: persistence still updates when intercept assignssubgroup.dataFiberState.FAILED,tree.writenot calledyarn test includestill green (no Include source change)Out of scope
events.ts/fiber.ts/ Include implementation#66inserts intopartialReload)Fiber.effecttracking means unload owns the real world