fix!: drop support for async preloaded queries

[XiNiHa]

This was breaking preloaded queries with variables

[changeset-bot]

🦋 Changeset detected

Latest commit: 3535cd8

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

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

[gemini-code-assist]

Hello @XiNiHa, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. The main goal of this PR, as indicated by the title and description, is to fix an issue where preloaded queries were breaking when used with variables. The approach taken is to drop support for asynchronous preloaded queries, which seems to be the source of the problem. This involves removing the use of SolidJS's createResource hook within the createPreloadedQuery primitive and adjusting the type signature and internal logic accordingly.

Highlights

• Drop Async Support: The PR explicitly removes support for passing a Promise for the preloadedQuery argument to createPreloadedQuery. This is a breaking change aimed at fixing issues with variable handling.
• Remove createResource Usage: The SolidJS createResource hook, previously used to handle the potential asynchronous nature of the input preloadedQuery, has been removed from the createPreloadedQuery implementation.
• Type Signature Update: The type definition for the preloadedQuery parameter in the createPreloadedQuery function has been updated to remove the MaybePromise wrapper, aligning the signature with the dropped asynchronous support.

Changelog

• .changeset/silver-cases-heal.md
  • Added a new changeset file documenting the fix and the breaking change (dropping async support for preloaded queries).
• src/primitives/createPreloadedQuery.ts
  • Removed the import of createResource from 'solid-js' (line 7).
  • Removed the MaybePromise type alias (line 16).
  • Updated the type signature of createPreloadedQuery to no longer accept a Promise for the preloadedQuery argument (line 17).
  • Removed the usage of the createResource hook (lines 23-26).
  • Updated internal logic to directly access the preloadedQuery value/signal instead of using the createResource result (lines 22, 23, 29, 30, 32).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/XiNiHa/solid-relay@30

commit: 3535cd8

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 27.90%. Comparing base (c0801c3) to head (3535cd8).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/primitives/createPreloadedQuery.ts	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   27.85%   27.90%   +0.04%     
==========================================
  Files          21       21              
  Lines        1411     1405       -6     
  Branches       70       70              
==========================================
- Hits          393      392       -1     
+ Misses       1018     1013       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gemini-code-assist]

Code Review

This pull request aims to fix an issue with preloaded queries when variables are involved by dropping support for asynchronous preloaded queries. The changes in src/primitives/createPreloadedQuery.ts correctly remove the asynchronous handling using createResource and simplify the logic by requiring preloadedQuery to be resolved synchronously.

This is a breaking change as indicated by fix! in the title and changeset, and users will need to adapt by ensuring their preloadedQuery is resolved before passing it to createPreloadedQuery.

While the code changes themselves look good for the intended purpose, there are a couple of points:

1. Documentation Update: The most critical follow-up is to update the API documentation for createPreloadedQuery to reflect that it no longer accepts a Promise<PreloadedQuery>. Outdated documentation for a breaking change can lead to significant confusion for users.
2. PR Description Clarity: The PR description "This was breaking preloaded queries with variables" is quite concise. For future reference and better understanding of the context behind this breaking change, it would be beneficial to briefly elaborate on how async preloading was causing issues with variables (e.g., timing issues, race conditions).

Overall, the code simplification is a positive step. My main recommendation is to ensure the documentation is updated to align with this breaking change.

Summary of Findings

• API Documentation Update Required: The function signature for createPreloadedQuery has changed, removing support for MaybePromise<PreloadedQuery>. This is a breaking change. The API documentation, specifically in docs/src/routes/api/functions/createPreloadedQuery.mdx (line 21 currently shows the old type), must be updated to reflect that preloadedQuery must be a direct PreloadedQuery<TQuery> or an accessor to one, not a Promise. This is crucial for users to understand the new API contract. Severity: Medium. (Not commented directly as the documentation file is not part of the diff).
• PR Description Clarity: The PR description is very brief. Expanding slightly on the nature of the bug (e.g., 'how async preloading broke queries with variables') would improve context for future maintainers understanding this breaking change. This is a low-severity suggestion and was not commented on directly due to review settings. (Not commented directly as this pertains to the PR body, not a specific code change in the diff).

Merge Readiness

The code changes in this pull request correctly implement the removal of asynchronous support for preloaded queries, which simplifies the logic and addresses the underlying issue. However, as this is a breaking change, it's very important that the API documentation is updated to reflect the new function signature for createPreloadedQuery before this PR is merged.

Therefore, I am requesting changes, primarily to ensure the documentation is brought in line with the code changes. Once the documentation is updated, this PR should be in good shape. As an AI, I am not authorized to approve pull requests; please ensure further review and approval by team members.