Skip to content

Conversation

dennev
Copy link
Contributor

@dennev dennev commented Aug 15, 2025

Summary

Address issue by replacing loading with _loading to avoid variable conflicts and restructure resource handling logic in createResource
It also addresses the specific error in another issue(#2132).

How did you test this change?

Try testing the code in the issue or a short-running async function block with a createResource as a fetcher.

Copy link

changeset-bot bot commented Aug 15, 2025

🦋 Changeset detected

Latest commit: 0223e3c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
solid-js Patch
test-integration Patch

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

@dennev dennev changed the title Fix/ssr create resource fix: handle promise resolution in createResource with improved inspection and serialization logic Aug 15, 2025
@coveralls
Copy link

coveralls commented Aug 15, 2025

Pull Request Test Coverage Report for Build 17024021319

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 85.276%

Totals Coverage Status
Change from base Build 16919208642: 0.004%
Covered Lines: 2339
Relevant Lines: 2673

💛 - Coveralls

p = new Promise(resolve => {
setTimeout(() => {
resolve(temp.then(val => handleResolvedValue(val, ctx, id)));
}, 300); // A safe number that won't throw an error.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh, no.

Copy link
Contributor Author

@dennev dennev Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've summarized it in #2534 and solidjs/solid-start#1941 (comment), if you're interested.
I apologize for the incorrect review request, I sent a review because I thought I had solved the problem in another way, but it wasn't feasible. Please forget about it.

Copy link
Contributor Author

@dennev dennev Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iacore I fixed the issue. You can check it out

@dennev dennev requested a review from iacore August 16, 2025 05:32
@dennev dennev force-pushed the fix/ssr-createResource branch 3 times, most recently from 4c8363e to 8b0f96e Compare August 16, 2025 07:25
@dennev dennev changed the title fix: handle promise resolution in createResource with improved inspection and serialization logic fix: Handling promise resolution in sync - inserting delays for resolved promises inside sync function blocks Aug 16, 2025
@dennev
Copy link
Contributor Author

dennev commented Aug 16, 2025

This commit is a proposal and is being submitted so that it can be part of some option. I understand people's concerns about timeouts, and I've summarized my considerations here.

@dennev dennev force-pushed the fix/ssr-createResource branch 2 times, most recently from 16b2e69 to e66ebc1 Compare August 17, 2025 08:17
@dennev
Copy link
Contributor Author

dennev commented Aug 17, 2025

I solved it by overriding loading.

@dennev dennev force-pushed the fix/ssr-createResource branch from e66ebc1 to d3f5257 Compare August 17, 2025 08:45
@dennev dennev changed the title fix: Handling promise resolution in sync - inserting delays for resolved promises inside sync function blocks fix: replace loading with _loading to avoid variable conflicts and restructure resource handling logic in createResource Aug 17, 2025
@dennev dennev force-pushed the fix/ssr-createResource branch 3 times, most recently from 68c3e98 to 9fa4ef9 Compare August 17, 2025 14:50
@dennev dennev changed the title fix: replace loading with _loading to avoid variable conflicts and restructure resource handling logic in createResource refactor resource handling: replace read.loading with _loading to avoid variable conflicts and restructure resource handling logic in createResource Aug 17, 2025
@dennev dennev force-pushed the fix/ssr-createResource branch from 9fa4ef9 to f0186ad Compare August 17, 2025 16:00
@dennev dennev changed the title refactor resource handling: replace read.loading with _loading to avoid variable conflicts and restructure resource handling logic in createResource Refactor: Rename loading to _loading and optimize createResource logic to prevent conflicts and improve resource management. Aug 17, 2025
@dennev dennev force-pushed the fix/ssr-createResource branch from f0186ad to 6e43f96 Compare August 17, 2025 16:03
… logic for improved conflict prevention and resource management.
@dennev dennev force-pushed the fix/ssr-createResource branch from 6e43f96 to 0223e3c Compare August 17, 2025 17:43
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