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

Use async fs methods #11

Merged
merged 3 commits into from
Oct 6, 2023
Merged

Use async fs methods #11

merged 3 commits into from
Oct 6, 2023

Conversation

zxdong262
Copy link
Contributor

@zxdong262 zxdong262 commented Oct 4, 2023

  • Tested in electerm, tsz/trz all works,
  • fs.open/fs.write/fs.read still use fs namespace function and sync function, but use await, so we can support pure async env like browser, since we need number type fileHandle, others functions use fs/promises.
  • Many tests can not pass, not if it is brought by this PR

src/nodefs.ts Outdated Show resolved Hide resolved
src/nodefs.ts Outdated Show resolved Hide resolved
src/nodefs.ts Outdated Show resolved Hide resolved
@zxdong262
Copy link
Contributor Author

有道理,我修改一下

for (const func of funcs) {
fs[func + 'Async'] = (...args) => {
return new Promise((resolve, reject) => {
fs[func](...args, (err, data) => {
Copy link
Member

Choose a reason for hiding this comment

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

当有多个参数时,这里一个 data 参数不够吧?
https://nodejs.org/api/fs.html#fsreadfd-buffer-offset-length-position-callback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

只有read的callback有多个返回,但是没有用到

Copy link
Member

Choose a reason for hiding this comment

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

多一个参数,就不需要这样一个一个地赋值了:
https://github.com/electerm/electerm/blob/e6c77eb94e865f511c4a6b9903cdbde09540e6ba/src/client/common/pre.js#L116-L118
可以直接返回另一个 buffer 。

Copy link
Member

Choose a reason for hiding this comment

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

@zxdong262 我提交了一个 commit 在这个分支上 https://github.com/trzsz/trzsz.js/tree/electerm
( 没有权限直接提交到这个 PR 中来 )

src/nodefs.ts Outdated Show resolved Hide resolved
@lonnywong lonnywong merged commit c57d360 into trzsz:main Oct 6, 2023
@lonnywong
Copy link
Member

@zxdong262 trzsz.js 发布了 v1.1.2,包含这个 PR #11

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.

2 participants