feat(nodejs): add WriteOptions for write methods

[bxb100]

close: #4782

[syhily]

hi, @syhily the code is implemented like this, any suggestion?

async function main() {
  const op = new Operator('s3', {
    root: '/test_opendal',
    bucket: 'your bucket name',
    region: 'your bucket region',
    endpoint: 'your endpoint',
    access_key_id: 'your access key id',
    secret_access_key: 'your secret access key',
  })
  
  await op.writeWith('test', 'Hello, World!')
    .contentType('text/plain')
    .apply()
}

Cool. I think this code snippet satisfied my needs.

[Xuanwo]

hi, @Xuanwo @sundy-li add devbox1 (nix wrapper) is a good idea or not?

This might be a good idea but how about moving them to a seperate PR?

[bxb100]

remove writeWith instead enhance original write methods

await operator.write(filename, c, {
      contentType,
      contentDisposition,
    })
// or
const writer = await operator.writer(filename, {
      contentType,
      contentDisposition,
    })
// or
operator.writeSync(filename, c, {
      contentType,
      contentDisposition,
    })
// or
const writer = operator.writerSync(filename, {
      contentType,
      contentDisposition,
    })

[bxb100]

hi, @suyanhanx , should I manually update the package.json version

[suyanhanx]

hi, @suyanhanx , should I manually update the package.json version

Please add those new features into upgrade.md as unreleased. That's enough. We will update the version when we release it.

[bxb100]

hi, @suyanhanx , should I manually update the package.json version

Please add those new features into upgrade.md as unreleased. That's enough. We will update the version when we release it.

Thanks replied, I will add it lately

[suyanhanx]

hi, @suyanhanx , should I manually update the package.json version

Please add those new features into upgrade.md as unreleased. That's enough. We will update the version when we release it.

Thanks replied, I will add it lately

This is not something you need to finish in this PR.😊A seperate PR is okay.

[Xuanwo]

Thanks for you effort. But OpXxx is not an existing concept at the binding side. How about using WriteOptions?

[bxb100]

IMHO, the OpWriteOptions concept corresponds to OpWrite, so I think it's better to use one object not OpWriteOptions, OpWriterOptions

the name starts with Op also because of this, but WriteOptions is ok for me

[Xuanwo]

IMHO, the OpWriteOptions concept corresponds to OpWrite, so I think it's better to use one object not OpWriteOptions, OpWriterOptions

Your understanding is correct. But there is a gap between rust core and nodejs binding.

Rust Core users have a concept called OpXxx like OpWrite and OpRead. But we never expose those APIs to nodejs binding. So from the nodejs users view, they don't know about OpXxx thus they can't understand what's OpXxx represents for.

So I think it's better to use WriteOptions without the Op prefix.

And for WriteOptions and WriterOptions, it's possible for the future to introduce different options inside write and writer options. So I want to seperate them for now instead of another breaking changes in the future.

[bxb100]

Thanks, that's a comprehensive explanation, I will fix it ASAP

[Xuanwo]

Maybe we need WriterOptions too?

[bxb100]

as I mentioned above, I think WriteOptions is enough

[Xuanwo]

It's better not to play with those APIs. What do you think about adding bunch of if-else directly?

---

This PR also makes me thinking about exporting WriteOptions from the rust core side. We can have a discussion there.

[bxb100]

you mean this?

         let mut writer = self.0.write_with(&path, c);
         if let Some(options) = options {
-            writer = writer.with(options);
+            if let Some(append) = options.append {
+                writer = writer.append(append);
+            }
+            if let Some(chunk) = options.chunk {
+                writer = writer.chunk(chunk.get_u64().1 as usize);
+            }
+            if let Some(ref content_type) = options.content_type {
+                writer = writer.content_type(content_type);
+            }
+            if let Some(ref content_disposition) = options.content_disposition {
+                writer = writer.content_disposition(content_disposition);
+            }
+            if let Some(ref cache_control) = options.cache_control {
+                writer = writer.cache_control(cache_control);
+            }
         }
         writer.await.map_err(format_napi_error)
     }

[Xuanwo]

Yes!

[Xuanwo]

We can polish here in the future if rust core exposes WriteOptions directly.

[Xuanwo]

Thanks a lot!