-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: fix a resource a type error when using async _resolved function #7426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 60bcdc8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
commit: |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
@maiieul please help me to review |
This commit refactors the preloader and bundle graph handling to improve performance and maintainability. Key changes include: - Removed `qwikloader-prefetch.ts` and `qwikloader-entry.ts` as they are no longer needed. - Updated the preloader configuration to use more descriptive variable names and optimized preload probabilities. - Changed the bundle graph output from `.js` to `.json` for better clarity and compression. - Added a new test for the preloader script to ensure its integrity and size. - Updated API Extractor configurations across multiple packages to use consistent naming and newline settings. These changes aim to enhance the efficiency of Qwik's preloading mechanism and ensure better maintainability of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain more why this is needed? A Resource is supposed to be a placeholder for a Promise, so I don't understand why it should also return a Promise
onResolved: (value: T) => JSXOutput | Promise<JSXOutput>; | ||
onPending?: () => JSXOutput | Promise<JSXOutput>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this is needed. The resource holds a Promise, but the Resource component should just render without a Promise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I know, it is correct for JavaScript, but it is not correct for TypeScript.
TypeScript will think that the function returns a Promise when async is added before the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you shouldn't add async before the function then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aaah ok now I get it, it's when the dev puts an async in the resource functions, ok. Hmm not sure if that's a good idea though - the render functions are just like inline components and we don't really support async inline components either. It works though :)
@Varixo thoughts?
What is it?
fixes: #7086
follow version 2 to fix this ts error
Description
Checklist
pnpm change