Skip to content

Commit

Permalink
Update porting notes about all InconsistentChildrenStateError cases
Browse files Browse the repository at this point in the history
  • Loading branch information
eyelidlessness committed May 16, 2024
1 parent afce3a9 commit 9a2c98a
Showing 1 changed file with 29 additions and 20 deletions.
49 changes: 29 additions & 20 deletions packages/scenario/test/repeat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,20 @@ describe('Tests ported from JavaRosa - repeats', () => {
*
* - It seems like the comment from JavaRosa on the above test may have
* been intended for this test? It seems to _almost describe_ this
* fixture. (Although to actually produce a consistent `count()` of 1, I
* think the `calculate` would need to be `count(..)`.)
*
* - Failure is an `InconsistentChildrenStateError`. Without diving into
* the cause, I've only ever seen this when experimenting with UI to
* exercise the engine API capability to add repeat instances at any
* index. It seemed pretty likely at the time that this was a Solid
* compilation bug, as it appeared that Solid's JSX transform triggered
* it. But this suggests that some (as yet undetermined) reactive
* property access may be implicated.
* fixture. (Although to actually produce a consistent `count()` of 1,
* I think the `calculate` would need to be `count(..)`.)
*
* - Failure is an `InconsistentChildrenStateError`. ~~Without diving
* into the cause, I've only ever seen this when experimenting with UI
* to exercise the engine API capability to add repeat instances at
* any index. It seemed pretty likely at the time that this was a
* Solid compilation bug, as it appeared that Solid's JSX transform
* triggered it. But this suggests that some (as yet undetermined)
* reactive property access may be implicated.~~ This error goes away
* if we unwrap the
* {@link https:/getodk/web-forms/blob/d40bd88ed8959bbe7fae5d28efc840c16ca50a72/packages/scenario/src/jr/Scenario.ts#L163-L173 | `createMemo` call}
* in Scenario.ts. That's a pretty (er) solid clue as to where the
* apparent bug must be.
*
* - A likely quick turnaround remedy would be a somewhat more involved
* children state mapping, with actual `nodeId` lookup rather than the
Expand All @@ -419,11 +423,11 @@ describe('Tests ported from JavaRosa - repeats', () => {
* - A more "correct" solution would almost certainly involve
* understanding how any particular reactive access could cause these
* states to go out of sync in the first place. It would likely also
* involve some investigation into whether the discrepancy is temporary
* and resolves after yielding to the event loop; this would implicate
* some aspect of Solid's reactive scheduling, which most of our
* reactive internals currently bypass (naively, trading CPU time for
* testability of the reactive bridge implementation).
* involve some investigation into whether the discrepancy is
* temporary and resolves after yielding to the event loop; this would
* implicate some aspect of Solid's reactive scheduling, which most of
* our reactive internals currently bypass (naively, trading CPU time
* for testability of the reactive bridge implementation).
*/
it.fails('updates relative repeat count, inside repeat', async () => {
const scenario = await Scenario.init(
Expand Down Expand Up @@ -538,9 +542,11 @@ describe('Tests ported from JavaRosa - repeats', () => {
/**
* **PORTING NOTES**
*
* - Another `InconsistentChildrenStateError`, another clue! This case is
* definitely triggered by the
* {@link Scenario.removeRepeat} call.
* - Another `InconsistentChildrenStateError`, another clue! ~~This
* case is definitely triggered by the {@link Scenario.removeRepeat}
* call.~~ This error goes away if we unwrap the
* {@link https:/getodk/web-forms/blob/d40bd88ed8959bbe7fae5d28efc840c16ca50a72/packages/scenario/src/jr/Scenario.ts#L163-L173 | `createMemo` call}
* in Scenario.ts.
* - Same thoughts on `nullValue()` -> blank/empty string check
*/
it.fails('updates that reference', async () => {
Expand Down Expand Up @@ -1600,8 +1606,11 @@ describe('Tests ported from JavaRosa - repeats', () => {
/**
* **PORTING NOTES**
*
* - Current failure is an `InconsistentChildrenStateError`, likely with
* similar root cause as other cases around repeat instance removal.
* - Current failure is an `InconsistentChildrenStateError` ~~, likely
* with similar root cause as other cases around repeat instance
* removal.~~ This error goes away if we unwrap the
* {@link https:/getodk/web-forms/blob/d40bd88ed8959bbe7fae5d28efc840c16ca50a72/packages/scenario/src/jr/Scenario.ts#L163-L173 | `createMemo` call}
* in Scenario.ts.
*
* - - -
*
Expand Down

0 comments on commit 9a2c98a

Please sign in to comment.