Skip to content
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

feat(useSSE): basic useSSE() implement #62

Merged
merged 5 commits into from
Apr 17, 2024

Conversation

MeetinaXD
Copy link
Contributor

这个提交实现了基本的 useSSE() hook,这个功能的实现依据:Server-sent events 发送请求

This PR implements the basic function of useSSE() hook, based on: Server-sent events Request

Copy link
Contributor

@JOU-amjs JOU-amjs left a comment

Choose a reason for hiding this comment

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

大的逻辑问题都不存在了,看你的编码能很快融入也是让我很惊讶的。

withCredentials,
interceptByGlobalResponded = trueValue,
/** abortLast = trueValue, */
immediate = trueValue
Copy link
Contributor

Choose a reason for hiding this comment

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

immediate该默认为false

// ! 暂时不支持指定 abortLast
const abortLast = trueValue;

let eventSource = $<EventSource | undefined>(undefinedValue, trueValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

建议eventSource使用useFlag$来标识引用,在react下使用useState时,只要更新这个状态值就会触发组件重新渲染,但eventSource对象是视图无关的,useFlag$更新就可以防止重新渲染

};

if (eventName === 'open') {
return ev(AlovaHookEventType.ScopedSQEvent);
Copy link
Contributor

Choose a reason for hiding this comment

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

这里建议可以新增一个事件名称,例如SSEOpenEventSSEErrorEvent这样的

const createSSEEvent = async (eventName: string, event: MessageEvent<any> | Event) => {
const es = _$(eventSource);
if (!es) {
throw new Error('eventSource is undefined');
Copy link
Contributor

Choose a reason for hiding this comment

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

我们每个hook都会创建一个assert函数,这里可以这样const paginationAssert = createAssert('useSSE');,报错信息比较明确范围

return ev(AlovaHookEventType.ScopedSQEvent);
}
if (eventName === 'error') {
return ev(AlovaHookEventType.ScopedSQErrorEvent, undefinedValue, new Error('SSE Error'));
Copy link
Contributor

Choose a reason for hiding this comment

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

如果是错误事件触发,是不是可以拿到一个错误对象,我们可以传这个错误对象,替代new Error('SSE Error')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

印象中 sse 的 error 事件返回的是 event 而不是 error 对象,所以没办法拿到。
使用下面的代码测试:

const es = new EventSource('https://google.com'); es.onerror = function(err){console.log('ERROR', err);}

结果:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

确实没有error对象或类似message的,那就按现在这样吧

Comment on lines 364 to 368
setTimeoutFn(() => {
if (immediate) {
connect();
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

这里没看明白,为什么还要setTimeout里执行呢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

调试时删漏了,现在不需要了。

之前 connect 没有在 onMounted 里执行,会导致 useSSE 还没返回就已经连接完了,事件也还没绑上😧

Comment on lines 131 to 134
if (!doNotCache) {
updateState(methodInstance, { data: transformedData });
}

Copy link
Contributor

Choose a reason for hiding this comment

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

我觉着这里应该是不需要缓存,这点设计里没提到,原因是sse的响应数据是多份的,缓存意义不大,而且我们之前说的两种场景都不需要缓存

// const mockOpenFn = jest.fn();

const Page = () => {
const { on, onOpen, data, readyState, send } = useSSE(poster, { immediate: false });
Copy link
Contributor

Choose a reason for hiding this comment

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

默认不请求的话,可能得把immediate参数去掉测试

@@ -0,0 +1,221 @@
import '@testing-library/jest-dom';
Copy link
Contributor

Choose a reason for hiding this comment

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

感觉单测覆盖较少,还有这些情况未测到

  1. interceptByGlobalResponded参数为true时,全局的onSuccess/onError/onComplete拦截器触发情况
  2. interceptByGlobalResponded为false时的情况
  3. 请求错误的情况
  4. 前一个sse还在未断开,又发起了新请求的情况

Comment on lines 312 to 317
if (!sendPromiseObject.current) {
sendPromiseObject.current = usePromise();
// open 后清除 promise 对象
sendPromiseObject.current.promise.finally(() => {
sendPromiseObject.current = undefinedValue;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

这里多次用了.current,我的话,我会习惯先使用const currentSendPromiseObj = sendPromiseObject.current引用下,然后其余地方使用currentSendPromiseObj更简洁些,只是说下想法,没什么问题

@MeetinaXD
Copy link
Contributor Author

上面提到的问题基本都解决了,我添加了许多关于 interceptByGlobalResponded 错误处理相关的测试。

至于上面提到的问题:

前一个sse还在未断开,又发起了新请求的情况

这个测试是存在的,可以看看:

useSSE.spec.tsx#L303

@JOU-amjs
Copy link
Contributor

JOU-amjs commented Apr 16, 2024

大概看了下应该是没什么问题了👋🏻

@MeetinaXD MeetinaXD merged commit 29f9ff9 into alovajs:main Apr 17, 2024
2 checks passed
Copy link

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants