Skip to content

Conversation

@mdynnl
Copy link
Contributor

@mdynnl mdynnl commented Nov 4, 2022

addresses #1323

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3396731709

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.155%

Totals Coverage Status
Change from base Build 3354507218: 0.0%
Covered Lines: 1282
Relevant Lines: 1368

💛 - Coveralls

@mdynnl
Copy link
Contributor Author

mdynnl commented Nov 4, 2022

what about

export function startTransition(fn: () => unknown): Promise<void> {
if (Transition && Transition.running) {
fn();
return Transition.done!;
}
const l = Listener;
const o = Owner;
return Promise.resolve().then(() => {
Listener = l;
Owner = o;
let t: TransitionState | undefined;
if (Scheduler || SuspenseContext) {
t =
Transition ||
(Transition = {
sources: new Set(),
effects: [],
promises: new Set(),
disposed: new Set(),
queue: new Set(),
running: true
});
t.done || (t.done = new Promise(res => (t!.resolve = res)));
t.running = true;
}
runUpdates(fn, false);
Listener = Owner = null;
return t ? t.done : undefined;
});
}

especially

should this go through solid's error handling?

@ryansolid
Copy link
Member

That one's probably fine. If the Transition is running it means it is running in a runUpdate further up. The problem with untrack is it messes with Listener/Owner and misses restoring it if it throws. Where this is handled higher up.

@ryansolid ryansolid merged commit 3c37e23 into solidjs:main Nov 5, 2022
@mdynnl mdynnl deleted the fix-untrack branch April 30, 2023 10:00
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3396731709

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.155%

Totals Coverage Status
Change from base Build 3354507218: 0.0%
Covered Lines: 1282
Relevant Lines: 1368

💛 - Coveralls

1 similar comment
@coveralls
Copy link

coveralls commented Nov 26, 2024

Pull Request Test Coverage Report for Build 3396731709

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.155%

Totals Coverage Status
Change from base Build 3354507218: 0.0%
Covered Lines: 1282
Relevant Lines: 1368

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants