aaronccasanova / aacc

Casa for my packages, projects, and experiments
11 stars 2 forks source link

[`cross-runtime`] [Feature requests] Tauri support & more methods #135

Open KaKi87 opened 5 months ago

KaKi87 commented 5 months ago

Hello,

It would be nice to add the following features to cross-runtime :

Additionally, it would be nice to :

If you'd like, I'm willing to make a PR for everything listed here, with JS and JSDoc, but not TS though.

Thanks

aaronccasanova commented 5 months ago

Hey @KaKi87 ๐Ÿ‘‹ Thanks for the feature requests!

  • Tauri support :

After a quick search, I'm unsure if Tauri is comparable to the other supported JavaScript runtimes in the library (e.g. Node.js, Deno, Bun). However, the linked fs modules do appear inline with the goal to normalize these types of APIs. I'll need more time to explore Tauri and suggest holding off on this feature until we determine the addition is inline with the Project Goals or if those goals should be refined.

  • A readLines method for reading a file line by line :
  • An exec method for running a command and returning an output :
  • A spawn method for running a command and returning a stream :

These all sound like great additions to the library ๐Ÿ‘

  • A writeBinaryFile method for writing binary files :
  • A readBinaryFile method for reading binary files : [...] Additionally, it would be nice to : have writeTextFile & readTextFile aliases for writeFile & readFile for consistency purposes ;

I'm not opposed to this, however, another goal of cross-runtime is to reduce the number of variations to do the same operation. I would prefer standardizing on using the encoding option for readFile and writeFile for defining the return value type. Thoughts?

use Bun.file().text() in readFile instead of relying on Bun's Node polyfill.

Agreed ๐Ÿ‘

If you'd like, I'm willing to make a PR for everything listed here, with JS and JSDoc, but not TS though.

Great to hear! Contributions are very welcome ๐Ÿš€ Feel free to start with readLine, exec, and spawn since we have alignment there. Happy to help with the API designs or discuss any of the feedback further.

KaKi87 commented 5 months ago

After a quick search, I'm unsure if Tauri is comparable to the other supported JavaScript runtimes in the library (e.g. Node.js, Deno, Bun).

It's not a runtime indeed, but it does match this library's use cases : manipulating files (and soon, processes) using a unified low overhead API.

I would prefer standardizing on using the encoding option for readFile and writeFile for defining the return value type. Thoughts?

Agreed, but then those methods would no longer have utf8 as default value, making this change a breaking one.

I'm okay with it if you are.

aaronccasanova commented 5 months ago

I would prefer standardizing on using the encoding option for readFile and writeFile for defining the return value type. Thoughts?

Agreed, but then those methods would no longer have utf8 as default value, making this change a breaking one.

I'm okay with it if you are.

Yes, I'm okay with the breaking change ๐Ÿ‘

I would prefer standardizing on using the encoding option

I was thinking on this more and wondering if the encoding convention can be challenged. My thought process was that if omitting encoding or setting undefined returns a Uint8Array, I'd also like there to be an explicit option for defining that return type (similar to Node.js readFile encoding: null option returning Buffer). That said, I think null is too abstract and would prefer an explicit string identifier (unrelated but similar to Node.js child_process encoding: 'buffer' option). However, in our context using encoding: 'uint8array' for example would be conflating the option since we're basically bypassing decoding and returning the raw binary data.

I'm curious what you think about deprecating encoding and introducing an as property for defining both the return type and optional encode/decode behavior. For example:

const foo = await readFile('foo.bin')
//    ^? Uint8Array

const bar = await readFile('bar.bin', { as: 'uint8array' })
//    ^? Uint8Array

const baz = await readFile('baz.txt', { as: 'utf8' })
//    ^? string

await writeFile('foo.bin', new TextEncoder().encode('data'))

await writeFile('bar.bin', new TextEncoder().encode('data'), { as: 'uint8array' })

await writeFile('baz.txt', 'data', { as: 'utf8' })
KaKi87 commented 5 months ago

I'd rather keep encoding for writeFile and pass it as-is to runtimes thus letting these handle it (less effort and bug risks for us), and only use as for readFile to convert outputs to the user's choice.

What do you think ?

aaronccasanova commented 5 months ago

I'd rather keep encoding for writeFile and pass it as-is to runtimes thus letting these handle it (less effort and bug risks for us), and only use as for readFile to convert outputs to the user's choice.

Okay, I agree. Using as in writeFile was a bit of a stretch for alignment and consistency with readFile. That said, I'd rather not diverge and have two different properties for similar operations. I suggest we keep encoding for both APIs where readFile omits or sets undefined to return a Uint8Array. For example:

const foo = await readFile('foo.bin')
//    ^? Uint8Array

const bar = await readFile('bar.bin', { encoding: undefined })
//    ^? Uint8Array

const baz = await readFile('baz.txt', { encoding: 'utf-8' })
//    ^? string

await writeFile('foo.bin', new TextEncoder().encode('data'))

await writeFile('bar.bin', new TextEncoder().encode('data'), { encoding: undefined })

// Note: Node.js silently ignores this unused `encoding` option, whereas, I lean towards throwing
await writeFile('expect-error.bin', new TextEncoder().encode('data'), { encoding: 'utf-8' })

await writeFile('baz.txt', 'data', { encoding: 'utf-8' })
KaKi87 commented 5 months ago

I suggest we keep encoding for both APIs

Agreed.

where readFile omits or sets undefined to return a Uint8Array.

Why not ArrayBuffer though ? Isn't that the most used type for file manipulation ?

Note: Node.js silently ignores this unused encoding option, whereas, I lean towards throwing

Okay.

aaronccasanova commented 5 months ago

where readFile omits or sets undefined to return a Uint8Array.

Why not ArrayBuffer though ? Isn't that the most used type for file manipulation ?

Good callout. I originally went with Uint8Array to avoid diverging too much from Node.js and Deno's readFile (note: while Node.js returns a Buffer it is a subclass of Uint8Array). That said, I'm happy to move forward with ArrayBuffer for its broader utility ๐Ÿ‘

KaKi87 commented 5 months ago

Alright, did you make a decision on Tauri ?

aaronccasanova commented 4 months ago

Yes, let's proceed with adding support for Tauri.

KaKi87 commented 4 months ago

Alright, here's one last thing โ€” I noticed that you statically import wrappers inside which you dynamically import runtime dependencies :

ยท
โ””โ”€โ”€ import { readFile, writeFile } from './index.js'
    โ”œโ”€โ”€ export { readFile } from './readFile.js'
    โ”‚   โ”œโ”€โ”€ { readFile } = import('node:fs/promises')
    โ”‚   โ”‚   โ””โ”€โ”€ readFile()
    โ”‚   โ””โ”€โ”€ Deno.readTextFile()
    โ””โ”€โ”€ export { writeFile } from './writeFile.js'
        โ”œโ”€โ”€ { writeFile } = import('node:fs/promises')
        โ”‚   โ””โ”€โ”€ writeFile()
        โ””โ”€โ”€ Deno.writeTextFile()

Although dynamic imports are unavoidable for being cross-runtime, I would like to suggest to dynamically import wrappers inside which statically import runtime dependencies :

ยท
โ””โ”€โ”€ import { readFile, writeFile } from './index.js'
    โ”œโ”€โ”€ { readFile, writeFile } = import('./node.js')
    โ”‚   โ”œโ”€โ”€ import { readFile, writeFile } from 'node:fs/promises'
    โ”‚   โ”‚   โ”œโ”€โ”€ readFile()
    โ”‚   โ”‚   โ””โ”€โ”€ writeFile()
    โ””โ”€โ”€ { readFile, writeFile } = import('./deno.js')
        โ”œโ”€โ”€ Deno.readTextFile()
        โ””โ”€โ”€ Deno.writeTextFile()

This opposite way would allow not to require dynamic imports when the user knows which runtime they're using, and directly import it :

import { readFile, writeFile } from './node.js'

What do you think ?

aaronccasanova commented 3 months ago

Great suggestion ๐Ÿ‘ I had a similar follow up task in mind to separate integrations into individual packages under the @cross-runtime/* scope e.g.

/**
 * @cross-runtime/types
 */
export type ReadFile = (
  path: string | URL,
  options: ReadFileOptions
) => Promise<ReadFileResult>

export type WriteFile = (
  path: string | URL,
  data: WriteFileData,
  options: WriteFileOptions
) => Promise<void>

/**
 * @cross-runtime/node
 */
import type {ReadFile, WriteFile} from '@cross-runtime/types'
import * as fs from 'node:fs/promises'

export const readFile: ReadFile = (path, options) => fs.readFile(...)
export const writeFile: WriteFile = (path, data, options) => fs.writeFile(...)

/**
 * @cross-runtime/deno
 */
import type {ReadFile, WriteFile} from '@cross-runtime/types'

export const readFile: ReadFile = (path, options) => options?.encoding
  ? Deno.readTextFile(...)
  : Deno.readFile(...)
export const writeFile: WriteFile = (path, data, options) => typeof data === 'string'
  ? Deno.writeTextFile(...)
  : Deno.writeFile(...)

/**
 * @cross-runtime/bun
 */
import type {ReadFile, WriteFile} from '@cross-runtime/types'
...

/**
 * cross-runtime
 */
export const {readFile, writeFile} = await (() => {
  if (isNode()) return import('@cross-runtime/node')
  if (isDeno()) return import('@cross-runtime/deno')
  if (isBun()) return import('@cross-runtime/bun')
  // ...
  throw new Error('Unsupported runtime')
})()
import {readFile, writeFile} from 'cross-runtime'
// or
import {readFile, writeFile} from '@cross-runtime/node'

That said, feel free to implement your suggestions as is and I can follow up with the restructure later. Otherwise, I'm happy to scaffold up these scoped packages now and we can continue these features after.

KaKi87 commented 3 months ago

I'd prefer not to do that though, because it complicates development unnecessarily, against the KISS principle.

To be honest, I already find a little frustrating that your project is part of a monorepo containing all your other projects that I have no interest in yet have to have on my computer ๐Ÿ˜