fix::notion-client - Replace ky with got || fix::react-notion-x - LazyImageFull.tsx updated
#646
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@mustaqimarifin collections are no longer loading: https://react-notion-x-minimal-demo-5sgdlsrby-saasify.vercel.app/9cb9716c93164c6c8b4cd0bac3879aeb |
i think the minimal demo doesn't import 'Collections', but the full demo does? Because the full demo does indeed load Collections on my end. Thx Left: Current React-Notion-X Test Suite || Right: Localhost s22.mp4 |
|
true, double checked and all good on the full example including my personal problematic page |
|
@transitive-bullshit can we merge this too? |
|
@transitive-bullshit kind reminder :) |
|
any update? |
|
there are too many unrelated changes to merge this PR. #653 was the main fix for collection issues |
|
i'm really hesitant to switch from |
totally understand. tbf, i tested it with native fetch and it works just as well. I'm not sure whats causing KY to stall out.. |
|
I also met this problem with NextJS App Router. #657 shows the details. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| image.addEventListener('load', resolve) | ||
| image.addEventListener = reject |
There was a problem hiding this comment.
There's a critical bug in the image loading error handling. Line 387 incorrectly assigns the reject function to the addEventListener method itself instead of adding an error event listener:
image.addEventListener = reject // incorrectThis should be changed to:
image.addEventListener('error', reject) // correctWithout this fix, image loading errors won't be properly caught and handled, potentially causing silent failures in the application.
| image.addEventListener('load', resolve) | |
| image.addEventListener = reject | |
| image.addEventListener('load', resolve) | |
| image.addEventListener('error', reject) |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| return got | ||
| .post(url, { | ||
| ...kyOptions, | ||
| json: body, | ||
| headers | ||
| }) | ||
| .json() |
There was a problem hiding this comment.
The original implementation separated the HTTP request and JSON parsing with a comment noting this was to avoid sporadic errors with body reuse:
// Separate awaits to avoid sporadic errors
const res = await ky.post(url, {...});
return res.json<T>();The new implementation with got.post().json() combines these steps, which might reintroduce the issue that was previously fixed. Consider maintaining the two-step approach:
const res = await got.post(url, {...});
return res.json();This preserves the original fix while still using the new HTTP client.
| return got | |
| .post(url, { | |
| ...kyOptions, | |
| json: body, | |
| headers | |
| }) | |
| .json() | |
| const res = await got | |
| .post(url, { | |
| ...kyOptions, | |
| json: body, | |
| headers | |
| }) | |
| return res.json() |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
|
Hey guys, Had some time and tested the various fetch packages out there and
Very few changes needed to use /* kyOptions: {
timeout: {
request: 60_000
},
...kyOptions,
searchParams: {
src: 'initial_load'
}
} */
oFetchOptions: {
timeout: 60_000,
...oFetchOptions,
params: {
src: 'initial_load'
}
}
/* const res = await ky.post(url, {
mode: 'no-cors',
...this._kyOptions,
...kyOptions,
json: body,
headers
})
return res.json<T>()
} */
const res = ofetch(url, {
method: "POST",
mode: 'no-cors',
...this._oFetchOptions,
...oFetchOptions,
body,
headers
})
return resLet me know if this works for everyone. |
…ocal dependencies. (Fix) Replace KY with OFetch. (Fix) Cleaned up unnceccesary and erroneous changes
Hi all @transitive-bullshit @rimonhanna @Euruson @zsolteszku , Yes, apologies for the messy PR. I've rebased the PR from the latest master with just the additions as originally intended. Happy to help if possible. |
Description
Replaced
KYwithGOTReact Lazy Images
react-lazy-imagesand placed updated code and types incomponents/lazy-images-full.tsxreact-intersection-observerandunionizeNotion Test Page ID
Can test with a local build of this fork with the examples in the monorepo.